From d28fbaa101595fb63269a1797e3223c9ad73bd59 Mon Sep 17 00:00:00 2001 From: Justin Starner Date: Thu, 4 Dec 2025 21:47:05 -0500 Subject: [PATCH 01/70] Refactor card deletion to use custom modal Replaces the standard turbo_confirm attribute with a custom dialog modal for a better user experience. - Removes `button_to_delete_card` helper in favor of inline markup. - Implements `keydown.esc->dialog#close:stop` to prevent the Escape key from inadvertently closing selected card. --- app/helpers/cards_helper.rb | 8 -------- app/views/cards/_messages.html.erb | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/app/helpers/cards_helper.rb b/app/helpers/cards_helper.rb index 581adc108..9b149b707 100644 --- a/app/helpers/cards_helper.rb +++ b/app/helpers/cards_helper.rb @@ -18,14 +18,6 @@ module CardsHelper &block end - def button_to_delete_card(card) - button_to card_path(card), - method: :delete, class: "btn txt-negative borderless txt-small", data: { turbo_frame: "_top", turbo_confirm: "Are you sure you want to permanently delete this card?" } do - concat(icon_tag("trash")) - concat(tag.span("Delete this card")) - end - end - def card_title_tag(card) title = [ card.title, diff --git a/app/views/cards/_messages.html.erb b/app/views/cards/_messages.html.erb index 402a39ec0..d4f6a6400 100644 --- a/app/views/cards/_messages.html.erb +++ b/app/views/cards/_messages.html.erb @@ -9,7 +9,22 @@ <% if Current.user.can_administer_card?(card) %> <% end %> <% end %> From fc3b63bb52a23fd4dcef2b8678defceb9bc876ad Mon Sep 17 00:00:00 2001 From: Justin Starner Date: Thu, 4 Dec 2025 21:49:28 -0500 Subject: [PATCH 02/70] Refactor board deletion to use custom modal Applies the custom dialog modal pattern to the board deletion workflow, matching the card deletion updates. --- app/views/boards/edit/_delete.html.erb | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/app/views/boards/edit/_delete.html.erb b/app/views/boards/edit/_delete.html.erb index 94a85d63f..47a7a3ed1 100644 --- a/app/views/boards/edit/_delete.html.erb +++ b/app/views/boards/edit/_delete.html.erb @@ -1,7 +1,16 @@ -<%= form_with model: board, class: "txt-align-center margin-block-start-auto", method: :delete do |form| %> - <%= form.button class: "btn txt-negative borderless txt-small", data: { turbo_confirm: "Are you sure you want to permanently delete this board and all the cards on it? This can't be undone." } do %> +
+ + +

Delete this board?

+

Are you sure you want to permanently delete this board and all the cards on it? This can't be undone.

+
+ + <%= button_to board_path(board), method: :delete, class: "btn txt-negative", data: { turbo_frame: "_top" } do %> + Delete board + <% end %> +
+ +
From 5cc6bba04023ad46f0d5121a428ef69ad6e7f54f Mon Sep 17 00:00:00 2001 From: Justin Starner Date: Thu, 4 Dec 2025 22:23:21 -0500 Subject: [PATCH 03/70] Move card deletion button to partial. --- app/views/cards/_delete.html.erb | 16 ++++++++++++++++ app/views/cards/_messages.html.erb | 17 +---------------- 2 files changed, 17 insertions(+), 16 deletions(-) create mode 100644 app/views/cards/_delete.html.erb diff --git a/app/views/cards/_delete.html.erb b/app/views/cards/_delete.html.erb new file mode 100644 index 000000000..5e6279478 --- /dev/null +++ b/app/views/cards/_delete.html.erb @@ -0,0 +1,16 @@ +
+ + +

Delete this card?

+

Are you sure you want to permanently delete this card?

+
+ + <%= button_to card_path(card), method: :delete, class: "btn txt-negative", data: { turbo_frame: "_top" } do %> + Delete card + <% end %> +
+ +
diff --git a/app/views/cards/_messages.html.erb b/app/views/cards/_messages.html.erb index d4f6a6400..a2784e72a 100644 --- a/app/views/cards/_messages.html.erb +++ b/app/views/cards/_messages.html.erb @@ -9,22 +9,7 @@ <% if Current.user.can_administer_card?(card) %>
-
- - -

Delete this card?

-

Are you sure you want to permanently delete this card?

-
- - <%= button_to card_path(card), method: :delete, class: "btn txt-negative", data: { turbo_frame: "_top" } do %> - Delete card - <% end %> -
- -
+ <%= render "cards/delete", card: card %>
<% end %> <% end %> From 437cc1e94fbe83667ddf4cb9d469c9c26101e080 Mon Sep 17 00:00:00 2001 From: luojiyin Date: Fri, 5 Dec 2025 12:01:36 +0800 Subject: [PATCH 04/70] chore: exclude test gems from production bundle --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 59cb8cfef..babd3b6d1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -24,7 +24,7 @@ RUN apt-get update -qq && \ ENV RAILS_ENV="production" \ BUNDLE_DEPLOYMENT="1" \ BUNDLE_PATH="/usr/local/bundle" \ - BUNDLE_WITHOUT="development" \ + BUNDLE_WITHOUT="development:test" \ LD_PRELOAD="/usr/local/lib/libjemalloc.so" # Throw-away build stage to reduce size of final image From 496851b255c6be2d50da1516321d702470934709 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 3 Dec 2025 23:10:18 -0800 Subject: [PATCH 05/70] Security: Web Push SSRF and IP range bypass Add SSRF protection for web push endpoints: - Resolve endpoint IP once and pin it for connection - Validate endpoints resolve to public IPs - Whitelist permitted push service hosts Add missing IP ranges to SsrfProtection: - 100.64.0.0/10 (Carrier-grade NAT, RFC6598) - 198.18.0.0/15 (Benchmark testing, RFC2544) Note: link-local (169.254.0.0/16) is already covered by ip.link_local? --- app/models/push/subscription.rb | 40 ++++++ app/models/ssrf_protection.rb | 4 +- config/initializers/web_push.rb | 10 +- lib/web_push/notification.rb | 6 +- .../push_subscriptions_controller_test.rb | 40 +++++- test/models/push/subscription_test.rb | 124 ++++++++++++++++++ test/models/ssrf_protection_test.rb | 60 +++++++++ 7 files changed, 276 insertions(+), 8 deletions(-) create mode 100644 test/models/push/subscription_test.rb create mode 100644 test/models/ssrf_protection_test.rb diff --git a/app/models/push/subscription.rb b/app/models/push/subscription.rb index 0b273b563..afd417367 100644 --- a/app/models/push/subscription.rb +++ b/app/models/push/subscription.rb @@ -1,14 +1,54 @@ class Push::Subscription < ApplicationRecord + PERMITTED_ENDPOINT_HOSTS = %w[ + fcm.googleapis.com + updates.push.services.mozilla.com + web.push.apple.com + notify.windows.com + ].freeze + belongs_to :account, default: -> { user.account } belongs_to :user + validates :endpoint, presence: true + validate :validate_endpoint_url + def notification(**params) WebPush::Notification.new( **params, badge: user.notifications.unread.count, endpoint: endpoint, + endpoint_ip: resolved_endpoint_ip, p256dh_key: p256dh_key, auth_key: auth_key ) end + + def resolved_endpoint_ip + return @resolved_endpoint_ip if defined?(@resolved_endpoint_ip) + @resolved_endpoint_ip = SsrfProtection.resolve_public_ip(endpoint_uri&.host) + end + + private + def endpoint_uri + @endpoint_uri ||= URI.parse(endpoint) if endpoint.present? + rescue URI::InvalidURIError + nil + end + + def validate_endpoint_url + if endpoint_uri.nil? + errors.add(:endpoint, "is not a valid URL") + elsif endpoint_uri.scheme != "https" + errors.add(:endpoint, "must use HTTPS") + elsif !permitted_endpoint_host? + errors.add(:endpoint, "is not a permitted push service") + elsif resolved_endpoint_ip.nil? + errors.add(:endpoint, "resolves to a private or invalid IP address") + end + end + + def permitted_endpoint_host? + host = endpoint_uri&.host&.downcase + PERMITTED_ENDPOINT_HOSTS.any? { |permitted| host&.end_with?(permitted) } + end end diff --git a/app/models/ssrf_protection.rb b/app/models/ssrf_protection.rb index f3df2511b..580d15713 100644 --- a/app/models/ssrf_protection.rb +++ b/app/models/ssrf_protection.rb @@ -4,7 +4,9 @@ module SsrfProtection DNS_RESOLUTION_TIMEOUT = 2 DISALLOWED_IP_RANGES = [ - IPAddr.new("0.0.0.0/8") # Broadcasts + IPAddr.new("0.0.0.0/8"), # "This" network (RFC1700) + IPAddr.new("100.64.0.0/10"), # Carrier-grade NAT (RFC6598) + IPAddr.new("198.18.0.0/15") # Benchmark testing (RFC2544) ].freeze def resolve_public_ip(hostname) diff --git a/config/initializers/web_push.rb b/config/initializers/web_push.rb index ac960468e..6914dae22 100644 --- a/config/initializers/web_push.rb +++ b/config/initializers/web_push.rb @@ -17,7 +17,15 @@ end module WebPush::PersistentRequest def perform - if @options[:connection] + endpoint_ip = @options[:endpoint_ip] + + if endpoint_ip + http = Net::HTTP.new(uri.host, uri.port, ipaddr: endpoint_ip) + http.use_ssl = true + http.ssl_timeout = @options[:ssl_timeout] unless @options[:ssl_timeout].nil? + http.open_timeout = @options[:open_timeout] unless @options[:open_timeout].nil? + http.read_timeout = @options[:read_timeout] unless @options[:read_timeout].nil? + elsif @options[:connection] http = @options[:connection] else http = Net::HTTP.new(uri.host, uri.port, *proxy_options) diff --git a/lib/web_push/notification.rb b/lib/web_push/notification.rb index d2865ff46..4c873fb48 100644 --- a/lib/web_push/notification.rb +++ b/lib/web_push/notification.rb @@ -1,13 +1,13 @@ class WebPush::Notification - def initialize(title:, body:, path:, badge:, endpoint:, p256dh_key:, auth_key:) + def initialize(title:, body:, path:, badge:, endpoint:, endpoint_ip:, p256dh_key:, auth_key:) @title, @body, @path, @badge = title, body, path, badge - @endpoint, @p256dh_key, @auth_key = endpoint, p256dh_key, auth_key + @endpoint, @endpoint_ip, @p256dh_key, @auth_key = endpoint, endpoint_ip, p256dh_key, auth_key end def deliver(connection: nil) WebPush.payload_send \ message: encoded_message, - endpoint: @endpoint, p256dh: @p256dh_key, auth: @auth_key, + endpoint: @endpoint, endpoint_ip: @endpoint_ip, p256dh: @p256dh_key, auth: @auth_key, vapid: vapid_identification, connection: connection, urgency: "high" diff --git a/test/controllers/users/push_subscriptions_controller_test.rb b/test/controllers/users/push_subscriptions_controller_test.rb index 9c77dec9d..c41d8313e 100644 --- a/test/controllers/users/push_subscriptions_controller_test.rb +++ b/test/controllers/users/push_subscriptions_controller_test.rb @@ -1,12 +1,15 @@ require "test_helper" class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest + PUBLIC_TEST_IP = "142.250.185.206" + setup do sign_in_as :david + stub_dns_resolution(PUBLIC_TEST_IP) end test "create new push subscription" do - subscription_params = { "endpoint" => "https://apple", "p256dh_key" => "123", "auth_key" => "456" } + subscription_params = { "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" } post user_push_subscriptions_path(users(:david)), params: { push_subscription: subscription_params }, headers: { "HTTP_USER_AGENT" => "Mozilla/5.0" } @@ -19,7 +22,7 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest test "touch existing subscription" do existing_subscription = users(:david).push_subscriptions.create!( - endpoint: "https://apple", + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", p256dh_key: "123", auth_key: "456" ) @@ -37,7 +40,7 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest test "destroy a push subscription" do subscription = users(:david).push_subscriptions.create!( - endpoint: "https://apple", + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", p256dh_key: "123", auth_key: "456" ) @@ -47,4 +50,35 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to user_push_subscriptions_path(users(:david)) end end + + test "rejects subscription with non-permitted endpoint" do + subscription_params = { "endpoint" => "https://attacker.example.com/steal", "p256dh_key" => "123", "auth_key" => "456" } + + assert_no_difference -> { Push::Subscription.count } do + post user_push_subscriptions_path(users(:david)), + params: { push_subscription: subscription_params } + end + + assert_response :unprocessable_entity + end + + test "rejects subscription with endpoint resolving to private IP" do + stub_dns_resolution("192.168.1.1") + + subscription_params = { "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" } + + assert_no_difference -> { Push::Subscription.count } do + post user_push_subscriptions_path(users(:david)), + params: { push_subscription: subscription_params } + end + + assert_response :unprocessable_entity + end + + private + def stub_dns_resolution(*ips) + dns_mock = mock("dns") + dns_mock.stubs(:each_address).multiple_yields(*ips) + Resolv::DNS.stubs(:open).yields(dns_mock) + end end diff --git a/test/models/push/subscription_test.rb b/test/models/push/subscription_test.rb new file mode 100644 index 000000000..9f4650614 --- /dev/null +++ b/test/models/push/subscription_test.rb @@ -0,0 +1,124 @@ +require "test_helper" + +class Push::SubscriptionTest < ActiveSupport::TestCase + PUBLIC_TEST_IP = "142.250.185.206" # google.com IP + + setup do + stub_dns_resolution(PUBLIC_TEST_IP) + end + + test "valid subscription with permitted endpoint" do + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert subscription.valid? + end + + test "rejects endpoint with non-https scheme" do + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "http://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_not subscription.valid? + assert_includes subscription.errors[:endpoint], "must use HTTPS" + end + + test "rejects endpoint with non-permitted host" do + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://attacker.example.com/webhook", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_not subscription.valid? + assert_includes subscription.errors[:endpoint], "is not a permitted push service" + end + + test "rejects endpoint that resolves to private IP" do + stub_dns_resolution("192.168.1.1") + + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_not subscription.valid? + assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address" + end + + test "rejects endpoint that resolves to loopback IP" do + stub_dns_resolution("127.0.0.1") + + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_not subscription.valid? + assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address" + end + + test "rejects endpoint that resolves to link-local IP (AWS IMDS)" do + stub_dns_resolution("169.254.169.254") + + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_not subscription.valid? + assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address" + end + + test "resolved_endpoint_ip returns pinned public IP" do + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_equal PUBLIC_TEST_IP, subscription.resolved_endpoint_ip + end + + test "accepts all permitted push service domains" do + permitted_endpoints = [ + "https://fcm.googleapis.com/fcm/send/token123", + "https://updates.push.services.mozilla.com/wpush/v2/token123", + "https://web.push.apple.com/QaBC123", + "https://wns2-db5p.notify.windows.com/w/?token=abc123" + ] + + permitted_endpoints.each do |endpoint| + subscription = Push::Subscription.new( + user: users(:david), + endpoint: endpoint, + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert subscription.valid?, "Expected #{endpoint} to be valid, got errors: #{subscription.errors.full_messages}" + end + end + + private + def stub_dns_resolution(*ips) + dns_mock = mock("dns") + dns_mock.stubs(:each_address).multiple_yields(*ips) + Resolv::DNS.stubs(:open).yields(dns_mock) + end +end diff --git a/test/models/ssrf_protection_test.rb b/test/models/ssrf_protection_test.rb new file mode 100644 index 000000000..d56f7390a --- /dev/null +++ b/test/models/ssrf_protection_test.rb @@ -0,0 +1,60 @@ +require "test_helper" + +class SsrfProtectionTest < ActiveSupport::TestCase + test "blocks loopback addresses" do + stub_dns_resolution("127.0.0.1") + assert_nil SsrfProtection.resolve_public_ip("localhost") + end + + test "blocks private 10.x.x.x addresses" do + stub_dns_resolution("10.0.0.1") + assert_nil SsrfProtection.resolve_public_ip("internal.example.com") + end + + test "blocks private 172.16.x.x addresses" do + stub_dns_resolution("172.16.0.1") + assert_nil SsrfProtection.resolve_public_ip("internal.example.com") + end + + test "blocks private 192.168.x.x addresses" do + stub_dns_resolution("192.168.1.1") + assert_nil SsrfProtection.resolve_public_ip("internal.example.com") + end + + test "blocks link-local addresses (AWS metadata endpoint)" do + stub_dns_resolution("169.254.169.254") + assert_nil SsrfProtection.resolve_public_ip("metadata.example.com") + end + + test "blocks carrier-grade NAT addresses" do + stub_dns_resolution("100.64.0.1") + assert_nil SsrfProtection.resolve_public_ip("cgnat.example.com") + end + + test "blocks benchmark testing addresses" do + stub_dns_resolution("198.18.0.1") + assert_nil SsrfProtection.resolve_public_ip("benchmark.example.com") + end + + test "blocks broadcast addresses" do + stub_dns_resolution("0.0.0.1") + assert_nil SsrfProtection.resolve_public_ip("broadcast.example.com") + end + + test "allows public addresses" do + stub_dns_resolution("93.184.216.34") + assert_equal "93.184.216.34", SsrfProtection.resolve_public_ip("example.com") + end + + test "returns first public IP when multiple addresses resolve" do + stub_dns_resolution("10.0.0.1", "93.184.216.34", "192.168.1.1") + assert_equal "93.184.216.34", SsrfProtection.resolve_public_ip("multi.example.com") + end + + private + def stub_dns_resolution(*ips) + dns_mock = mock("dns") + dns_mock.stubs(:each_address).multiple_yields(*ips) + Resolv::DNS.stubs(:open).yields(dns_mock) + end +end From 7b6dfeced445f6f86d4fe9859e647d6c4e87d8ed Mon Sep 17 00:00:00 2001 From: user Date: Fri, 5 Dec 2025 14:59:39 +0900 Subject: [PATCH 06/70] Update --font-sans variable for better cross-platform compatibility --- app/assets/stylesheets/_global.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/_global.css b/app/assets/stylesheets/_global.css index 414d4c696..7f9e567a6 100644 --- a/app/assets/stylesheets/_global.css +++ b/app/assets/stylesheets/_global.css @@ -10,7 +10,7 @@ --block-space-double: calc(var(--block-space) * 2); /* Text */ - --font-sans: system-ui; + --font-sans: -apple-system, BlinkMacSystemFont, sans-serif; --font-serif: ui-serif, serif; --font-mono: ui-monospace, monospace; From 21f3f72647a1959429df701cb130b48363e5acee Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 4 Dec 2025 23:17:21 -0800 Subject: [PATCH 07/70] Immediate avatar and embed variants Process variants synchronously on attachment to close the window between image upload and variant availability, guaranteeing that we won't have lazy variant processing attempts in GET requests. Tradeoff is that we do variant processing in upload requests, which is actually desirable. We're working with images that should take milliseconds to resize given that we'll already have the file on hand. References https://github.com/rails/rails/pull/51951 --- Gemfile.lock | 2 +- Gemfile.saas.lock | 6 ++--- app/models/concerns/attachments.rb | 9 +++---- app/models/user/avatar.rb | 2 +- config/initializers/action_text.rb | 2 +- .../users/avatars_controller_test.rb | 8 +++++-- .../notification/bundle_mailer_test.rb | 3 ++- test/models/comment_test.rb | 20 ++++++++++++++++ test/models/user/avatar_test.rb | 24 +++++++++++++++++-- 9 files changed, 59 insertions(+), 17 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index b9c3489b5..59e79ec08 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -6,7 +6,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: 3c3f6c8a253c8a0f346695374f927c9ab32fbefb + revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0 branch: main specs: actioncable (8.2.0.alpha) diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index 5cd308155..328158c0d 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -75,7 +75,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: 690ec8898318b8f50714e86676353ebe1551261e + revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0 branch: main specs: actioncable (8.2.0.alpha) @@ -292,7 +292,7 @@ GEM actionview (>= 7.0.0) activesupport (>= 7.0.0) jmespath (1.6.2) - json (2.16.0) + json (2.17.1) jwt (3.1.2) base64 kamal (2.9.0) @@ -552,7 +552,7 @@ GEM ostruct stimulus-rails (1.3.4) railties (>= 6.0.0) - stringio (3.1.8) + stringio (3.1.9) thor (1.4.0) thruster (0.1.16) thruster (0.1.16-aarch64-linux) diff --git a/app/models/concerns/attachments.rb b/app/models/concerns/attachments.rb index 1d1c2c4d6..bcc7303ae 100644 --- a/app/models/concerns/attachments.rb +++ b/app/models/concerns/attachments.rb @@ -1,13 +1,10 @@ module Attachments extend ActiveSupport::Concern - # The variants we use must all declared here so that we can preprocess them. + # Variants used by ActionText embeds. Processed immediately on attachment to avoid + # read replica issues (lazy variants would attempt writes on read replicas). # - # If they are not preprocessed, then Rails will attempt to transform the image on-the-fly when - # they are first viewed, which may be on the read replica where writing to the database is not - # allowed. Chaos will ensue if that happens. - # - # These variants are patched into ActionText::RichText in config/initializers/action_text.rb + # Patched into ActionText::RichText in config/initializers/action_text.rb VARIANTS = { # vipsthumbnail used to create sized image variants has a intent setting to manage colors during # resize. By setting an invalid intent value the gif-incompatible intent filtering is skipped and diff --git a/app/models/user/avatar.rb b/app/models/user/avatar.rb index a4b2ae631..c635edddb 100644 --- a/app/models/user/avatar.rb +++ b/app/models/user/avatar.rb @@ -5,7 +5,7 @@ module User::Avatar included do has_one_attached :avatar do |attachable| - attachable.variant :thumb, resize_to_fill: [ 256, 256 ] + attachable.variant :thumb, resize_to_fill: [ 256, 256 ], process: :immediately end validate :avatar_content_type_allowed diff --git a/config/initializers/action_text.rb b/config/initializers/action_text.rb index b2724b1c7..eee87de0d 100644 --- a/config/initializers/action_text.rb +++ b/config/initializers/action_text.rb @@ -7,7 +7,7 @@ module ActionText # This overrides the default :embeds association! has_many_attached :embeds do |attachable| ::Attachments::VARIANTS.each do |variant_name, variant_options| - attachable.variant variant_name, variant_options.merge(preprocessed: true) + attachable.variant variant_name, **variant_options, process: :immediately end end end diff --git a/test/controllers/users/avatars_controller_test.rb b/test/controllers/users/avatars_controller_test.rb index 3ded884b8..be4532448 100644 --- a/test/controllers/users/avatars_controller_test.rb +++ b/test/controllers/users/avatars_controller_test.rb @@ -27,7 +27,9 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest end test "show own image redirects to the blob url" do - users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + # Create blob separately to ensure file is uploaded before variant processing + blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + users(:david).avatar.attach(blob) assert users(:david).avatar.attached? get user_avatar_path(users(:david)) @@ -36,7 +38,9 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest end test "show other image redirects to the blob url" do - users(:kevin).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + # Create blob separately to ensure file is uploaded before variant processing + blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + users(:kevin).avatar.attach(blob) assert users(:kevin).avatar.attached? get user_avatar_path(users(:kevin)) diff --git a/test/mailers/notification/bundle_mailer_test.rb b/test/mailers/notification/bundle_mailer_test.rb index fbc4274c9..63647e68a 100644 --- a/test/mailers/notification/bundle_mailer_test.rb +++ b/test/mailers/notification/bundle_mailer_test.rb @@ -22,11 +22,12 @@ class Notification::BundleMailerTest < ActionMailer::TestCase end test "renders avatar with external image URL when avatar is attached" do - @user.avatar.attach( + blob = ActiveStorage::Blob.create_and_upload!( io: File.open(Rails.root.join("test", "fixtures", "files", "avatar.png")), filename: "avatar.png", content_type: "image/png" ) + @user.avatar.attach(blob) create_notification(@user) diff --git a/test/models/comment_test.rb b/test/models/comment_test.rb index c519f8fbd..13a5e36c3 100644 --- a/test/models/comment_test.rb +++ b/test/models/comment_test.rb @@ -4,4 +4,24 @@ class CommentTest < ActiveSupport::TestCase setup do Current.session = sessions(:david) end + + test "rich text embed variants are processed immediately on attachment" do + comment = cards(:logo).comments.create!(body: "Check this out") + comment.body.body.attachables # force load + + blob = ActiveStorage::Blob.create_and_upload! \ + io: File.open(file_fixture("moon.jpg")), + filename: "moon.jpg", + content_type: "image/jpeg" + + comment.body.body = ActionText::Content.new(comment.body.body.to_html).append_attachables(blob) + comment.save! + + embed = comment.body.embeds.sole + + Attachments::VARIANTS.each_key do |variant_name| + variant = embed.variant(variant_name) + assert variant.processed?, "Expected #{variant_name} variant to be processed immediately" + end + end end diff --git a/test/models/user/avatar_test.rb b/test/models/user/avatar_test.rb index 96ea5bac7..58b78969c 100644 --- a/test/models/user/avatar_test.rb +++ b/test/models/user/avatar_test.rb @@ -2,7 +2,8 @@ require "test_helper" class User::AvatarTest < ActiveSupport::TestCase test "avatar_thumbnail returns variant for variable images" do - users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + users(:david).avatar.attach(blob) assert users(:david).avatar.variable? assert_equal users(:david).avatar.variant(:thumb).blob, users(:david).avatar_thumbnail.blob @@ -16,7 +17,8 @@ class User::AvatarTest < ActiveSupport::TestCase end test "allows valid image content types" do - users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg") + blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg", content_type: "image/jpeg") + users(:david).avatar.attach(blob) assert users(:david).valid? end @@ -27,4 +29,22 @@ class User::AvatarTest < ActiveSupport::TestCase assert_not users(:david).valid? assert_includes users(:david).errors[:avatar], "must be a JPEG, PNG, GIF, or WebP image" end + + test "thumb variant is processed immediately on attachment" do + # Create blob separately to ensure file is uploaded before variant processing. + # + # Root cause: When ActiveStorage::Record uses `connects_to` for read replica support + # (as in SAAS mode), it creates a separate connection pool from application models. + # Since after_commit callbacks are tracked per connection pool, the callback order + # between User's pool (upload) and Attachment's pool (create_variants) isn't guaranteed. + # In MySQL/SAAS mode, the Attachment callback fires before the file is uploaded. + blob = ActiveStorage::Blob.create_and_upload!( + io: File.open(file_fixture("avatar.png")), + filename: "avatar.png", + content_type: "image/png" + ) + users(:david).avatar.attach(blob) + + assert users(:david).avatar.variant(:thumb).processed? + end end From 67f36886a1b85ca752c0dcefb77955814eda010d Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 27 Nov 2025 06:53:03 +0100 Subject: [PATCH 08/70] Update lexxy to latest Color highlighter, more code languages... --- Gemfile.lock | 2 +- app/assets/stylesheets/_global.css | 21 +++++++ app/assets/stylesheets/lexxy.css | 65 ++++++++++++++++++++ app/assets/stylesheets/rich-text-content.css | 4 ++ 4 files changed, 91 insertions(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index b9c3489b5..57f59eabb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -237,7 +237,7 @@ GEM logger (~> 1.6) letter_opener (1.10.0) launchy (>= 2.2, < 4) - lexxy (0.1.20.beta) + lexxy (0.1.22.beta) rails (>= 8.0.2) lint_roller (1.1.0) logger (1.7.0) diff --git a/app/assets/stylesheets/_global.css b/app/assets/stylesheets/_global.css index 414d4c696..13988f027 100644 --- a/app/assets/stylesheets/_global.css +++ b/app/assets/stylesheets/_global.css @@ -219,6 +219,27 @@ --color-card-7: oklch(var(--lch-purple-medium)); --color-card-8: oklch(var(--lch-pink-medium)); + /* Colors: Highlighter */ + --highlight-1: rgb(136, 118, 38); + --highlight-2: rgb(185, 94, 6); + --highlight-3: rgb(207, 0, 0); + --highlight-4: rgb(216, 28, 170); + --highlight-5: rgb(144, 19, 254); + --highlight-6: rgb(5, 98, 185); + --highlight-7: rgb(17, 138, 15); + --highlight-8: rgb(148, 82, 22); + --highlight-9: rgb(102, 102, 102); + + --highlight-bg-1: rgba(229, 223, 6, 0.3); + --highlight-bg-2: rgba(255, 185, 87, 0.3); + --highlight-bg-3: rgba(255, 118, 118, 0.3); + --highlight-bg-4: rgba(248, 137, 216, 0.3); + --highlight-bg-5: rgba(190, 165, 255, 0.3); + --highlight-bg-6: rgba(124, 192, 252, 0.3); + --highlight-bg-7: rgba(140, 255, 129, 0.3); + --highlight-bg-8: rgba(221, 170, 123, 0.3); + --highlight-bg-9: rgba(200, 200, 200, 0.3); + /* Colors: Syntax highlighting */ --color-code-token__att: oklch(var(--lch-blue-dark)); --color-code-token__comment: oklch(var(--lch-ink-medium)); diff --git a/app/assets/stylesheets/lexxy.css b/app/assets/stylesheets/lexxy.css index e3f8f8cbd..84225a55b 100644 --- a/app/assets/stylesheets/lexxy.css +++ b/app/assets/stylesheets/lexxy.css @@ -167,6 +167,71 @@ flex: 1; } + :where(.lexxy-highlight-dialog) { + .lexxy-highlight-dialog-content { + display: flex; + flex-direction: column; + gap: 0.75ch; + + [data-button-group] { + display: flex; + flex-direction: row; + flex-wrap: wrap; + gap: 0.5ch; + } + + button { + appearance: none; + background: var(--color-canvas); + block-size: 3.5ch; + border: none; + border-radius: 0.5ch; + color: var(--color-ink); + cursor: pointer; + display: grid; + font-size: inherit; + inline-size: 3.5ch; + place-items: center; + position: relative; + outline: none; + + &:after { + align-self: center; + content: "Aa"; + display: inline-block; + font-family: system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; + position: absolute; + inset-block-start: 0; + inset-block-end: 0; + inset-inline-end: 0; + inset-inline-start: 0; + } + + &:hover { + opacity: 0.8; + } + + &[aria-pressed="true"] { + box-shadow: 0 0 0 1px var(--color-canvas), 0 0 0 3px var(--color-link); + + &:after { + content: "✓"; + } + } + } + + .lexxy-highlight-dialog-reset { + inline-size: 100%; + + &[disabled] { + display: none; + } + + &:after { display: none; } + } + } + } + /* Based on .input, .input--select */ .lexxy-code-language-picker { -webkit-appearance: none; diff --git a/app/assets/stylesheets/rich-text-content.css b/app/assets/stylesheets/rich-text-content.css index 2d465c935..9f106fbd6 100644 --- a/app/assets/stylesheets/rich-text-content.css +++ b/app/assets/stylesheets/rich-text-content.css @@ -40,6 +40,10 @@ text-decoration: line-through; } + :where(mark, .lexxy-content__highlight) { + background-color: transparent; + color: inherit; + } :where(p, blockquote) { letter-spacing: -0.005ch; From 48ef7d0b983443c0ef8af101a8cc640717866343 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Fri, 5 Dec 2025 11:24:52 +0100 Subject: [PATCH 09/70] Update Lexxy and hide the highlighter for now --- Gemfile.lock | 2 +- Gemfile.saas.lock | 2 +- app/assets/stylesheets/lexxy.css | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 57f59eabb..43d4e718b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -237,7 +237,7 @@ GEM logger (~> 1.6) letter_opener (1.10.0) launchy (>= 2.2, < 4) - lexxy (0.1.22.beta) + lexxy (0.1.23.beta) rails (>= 8.0.2) lint_roller (1.1.0) logger (1.7.0) diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index 5cd308155..39af96f95 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -313,7 +313,7 @@ GEM logger (~> 1.6) letter_opener (1.10.0) launchy (>= 2.2, < 4) - lexxy (0.1.20.beta) + lexxy (0.1.23.beta) rails (>= 8.0.2) lint_roller (1.1.0) logger (1.7.0) diff --git a/app/assets/stylesheets/lexxy.css b/app/assets/stylesheets/lexxy.css index 84225a55b..a737cc590 100644 --- a/app/assets/stylesheets/lexxy.css +++ b/app/assets/stylesheets/lexxy.css @@ -152,6 +152,11 @@ } } + /* Temporarily hide the highlighter button until we tackle the styles */ + [name="strikethrough"] + .lexxy-editor__toolbar-dropdown { + display: none; + } + .lexxy-editor__toolbar-overflow-menu { background-color: var(--color-canvas); border-radius: 0.5ch; From 80a0eb3a4d90df6653695ac65965729c322b0222 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Fri, 5 Dec 2025 11:30:58 +0100 Subject: [PATCH 10/70] Fix system test --- test/system/smoke_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/system/smoke_test.rb b/test/system/smoke_test.rb index e4bc9cae9..539921b93 100644 --- a/test/system/smoke_test.rb +++ b/test/system/smoke_test.rb @@ -24,7 +24,7 @@ class SmokeTest < ApplicationSystemTestCase within("form lexxy-editor figure.attachment[data-content-type='image/jpeg']") do assert_selector "img[src*='/rails/active_storage']" - assert_selector "figcaption input[placeholder='moon.jpg']" + assert_selector "figcaption textarea[placeholder='moon.jpg']" end click_on "Post" From 1ac5867d5cea26e8ff8a27ca4c5412499c43231f Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Fri, 5 Dec 2025 11:41:11 +0100 Subject: [PATCH 11/70] These are textareas now --- app/assets/stylesheets/lexxy.css | 15 +++++++++++++++ app/assets/stylesheets/rich-text-content.css | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/lexxy.css b/app/assets/stylesheets/lexxy.css index a737cc590..939a3fb5e 100644 --- a/app/assets/stylesheets/lexxy.css +++ b/app/assets/stylesheets/lexxy.css @@ -237,6 +237,21 @@ } } + .attachment__caption { + textarea { + background-color: var(--input-background, transparent); + border-radius: var(--input-border-radius, 0.5em); + border: var(--input-border-size, 1px) solid var(--input-border-color, var(--color-ink-medium)); + color: var(--input-color, var(--color-ink)); + font-size: max(16px, 1em); + inline-size: 100%; + line-height: inherit; + max-inline-size: 100%; + padding: var(--input-padding, 0.5em 0.8em); + resize: none; + } + } + /* Based on .input, .input--select */ .lexxy-code-language-picker { -webkit-appearance: none; diff --git a/app/assets/stylesheets/rich-text-content.css b/app/assets/stylesheets/rich-text-content.css index 9f106fbd6..ca3715cba 100644 --- a/app/assets/stylesheets/rich-text-content.css +++ b/app/assets/stylesheets/rich-text-content.css @@ -205,7 +205,7 @@ color: color-mix(in oklch, var(--color-ink) 66%, transparent); font-size: var(--text-small); - .input { + textarea { --input-border-radius: 0.3em; --input-border-size: 0; --input-padding: 0; From 04153038f02bd5786153a63422538a737b4bf55f Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 5 Dec 2025 13:21:32 +0000 Subject: [PATCH 12/70] Document deployment - Update the README with detailed information about how to deploy a Fizzy instance - Reduce the example deploy config - Add example SMTP configuration in production.rb --- README.md | 125 +++++++++++++++++++++++++----- config/deploy.yml | 113 +++++++-------------------- config/environments/production.rb | 15 ++++ 3 files changed, 148 insertions(+), 105 deletions(-) diff --git a/README.md b/README.md index 8a7877326..613e17ecc 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,95 @@ This is the source code of [Fizzy](https://fizzy.do/), the Kanban tracking tool for issues and ideas by [37signals](https://37signals.com). + +## Deploying Fizzy + +If you'd like to run Fizzy on your own server, we recommend deploying it with [Kamal](https://kamal-deploy.org/). +Kamal makes it easier to set up a bare server, copy the application to it, and manage the configuration settings that it uses. + +(Kamal is also what we use to deploy Fizzy at 37signals. If you're curious about what our deployment configuration looks like, you can find it inside [`fizzy-saas`](https://github.com/basecamp/fizzy-saas).) + +This repo contains a starter deployment file that you can modify for your own specific use. That file lives at [config/deploy.yml](config/deploy.yml), which is the default place where Kamal will look for it. + +The steps to configure your very own Fizzy are: + +1. Fork the repo +2. Edit few things in config/deploy.yml, .kamal/secrets, and config/environments/production.rb +3. Run `kamal setup` to do your first deploy. + +We'll go through each of these in turn. + +### Fork the repo + +To make it easy to customise Fizzy's settings for your own instance, you should start by creating your own GitHub fork of the repo. +That allows you to commit your changes, and track them over time. +You can always re-sync your fork to pick up new changes from the main repo over time. + +Once you've got your fork ready, run `bin/setup` from within it, to make sure everything is installed. + +### Editing the configuration + +The config/deploy.yml has been mostly set up for you, but you'll need to fill out some sections that are specific to your instance. +To get started, the parts you need to change are all in the "About your deployment" section. +We've added comments to that file to highlight what each setting needs to be, but the main ones are: + +- `servers/web`: Enter the hostname of the server you're deploying to here. This should be an address that you can access via `ssh`. +- `ssh/user`: If you access your server a `root` you can leave this alone; if you use a different user, set it here. +- `proxy/ssl` and `proxy/host`: Kamal can set up SSL certificates for you automatically. To enable that, set the hostname again as `host`. If you don't want SSL for some reason, you can set `ssl: false` to turn it off. +- `env/clear/MAILER_FROM_ADDRESS`: This is the email address that Fizzy will send emails from. It should usually be an address from the same domain where you're running Fizzy. + +Fizzy also requires a few environment variables to be set up, some of which contain secrets. +The simplest way to do this is to put them in a file called `.kamal/secrets`. +Because this file will contain secret credentials, it's important that you DON'T CHECK THIS FILE INTO YOUR REPO! You can add the filename to `.gitignore` to ensure you don't commit this file accidentally. + +If you use a password manager like 1Password, you can also opt to keep your secrets there instead. +Refer to the [Kamal documentation](https://kamal-deploy.org/docs/configuration/environment-variables/#secrets) for more information about how to do that. + +To store your secrets, create the file `.kamal/secrets` and enter something like the following: + +``` +SECRET_KEY_BASE=12345 +VAPID_PUBLIC_KEY=something +VAPID_PRIVATE_KEY=somethingelse +SMTP_USERNAME=email-provider-username +SMTP_PASSWORD=email-provider-password +``` + +The values you enter here will be specific to you, and you can get or create them as follows: + +- `SECRET_KEY_BASE` should be a long, random secret. You can run `bin/rails secret` to create a suitable value for this. +- `VAPID_PUBLIC_KEY` & `VAPID_PRIVATE_KEY` are a pair of credentials that are used for sending notifications. You can create your own keys by starting a development console with: + + bin/rails c + + And then run the following to create a new pair of keys: + + ```ruby + vapid_key = WebPush.generate_key + + puts "VAPID_PRIVATE_KEY=#{vapid_key.private_key}" + puts "VAPID_PUBLIC_KEY=#{vapid_key.public_key}" + ``` + +- `SMTP_USERNAME`/`SMTP_PASSWORD` are credentials you should get from your email provider. + +Lastly, you'll need to set up the rest of your email configuration in `config/environments/production.rb`. There is an example configuration in comments at the top of that file. The actual settings you use here will depend on your email provider, but in most cases will look similar to that section, so you can uncomment it and edit to suit. Note that it will use the `SMTP_USERNAME` and `SMTP_PASSWORD` values you entered in your secrets. + +Once you've made all those changes, commit them to your fork so they're saved. + +### Deploy Fizzy! + +You can now do your first deploy by running: + + bin/kamal setup + +This will set up Docker (if needed), build your Fizzy app container, configure it, and start it running. + +After the first deploy is done, any subsequent steps won't need to do that initial setup. So for future deploys you can just run: + + bin/kamal deploy + + ## Development ### Setting up @@ -23,6 +112,22 @@ You'll be able to access the app in development at http://fizzy.localhost:3006. To login, enter `david@example.com` and grab the verification code from the browser console to sign in. +### Web Push Notifications + +Fizzy uses VAPID (Voluntary Application Server Identification) keys to send browser push notifications. For notifications to work in development you'll need to generate a key pair and set these environment variables: + +- `VAPID_PRIVATE_KEY` +- `VAPID_PUBLIC_KEY` + +Generate them with the `web-push` gem: + +```ruby +vapid_key = WebPush.generate_key + +puts "VAPID_PRIVATE_KEY=#{vapid_key.private_key}" +puts "VAPID_PUBLIC_KEY=#{vapid_key.public_key}" +``` + ### Running tests For fast feedback loops, unit tests can be run with: @@ -54,25 +159,6 @@ You can enable or disable [`letter_opener`](https://github.com/ryanb/letter_open Under the hood, this will create or remove `tmp/email-dev.txt`. -## Deployment - -We recommend [Kamal](https://kamal-deploy.org/) for deploying Fizzy. This project comes with a vanilla Rails template. You can find our production setup in [`fizzy-saas`](https://github.com/basecamp/fizzy-saas). - -### Web Push Notifications - -Fizzy uses VAPID (Voluntary Application Server Identification) keys to send browser push notifications. You'll need to generate a key pair and set these environment variables: - -- `VAPID_PRIVATE_KEY` -- `VAPID_PUBLIC_KEY` - -Generate them with the `web-push` gem: - -```ruby -vapid_key = WebPush.generate_key - -puts "VAPID_PRIVATE_KEY=#{vapid_key.private_key}" -puts "VAPID_PUBLIC_KEY=#{vapid_key.public_key}" -``` ## SaaS gem @@ -85,6 +171,7 @@ This gem depends on some private git repositories and it is not meant to be used We welcome contributions! Please read our [style guide](STYLE.md) before submitting code. + ## License Fizzy is released under the [O'Saasy License](LICENSE.md). diff --git a/config/deploy.yml b/config/deploy.yml index be1e3c21e..0e855727d 100644 --- a/config/deploy.yml +++ b/config/deploy.yml @@ -1,65 +1,45 @@ -# Name of your application. Used to uniquely configure containers. +# Name of this app service: fizzy - -# Name of the container image (use your-user/app-name on external registries). image: fizzy -# Deploy to these servers. + +#-- About your deployment --# + +# Where to deploy fizzy servers: web: - - 192.168.0.1 - # job: - # hosts: - # - 192.168.0.1 - # cmd: bin/jobs + - fizzy.example.com # Set your server name here -# Enable SSL auto certification via Let's Encrypt and allow for multiple apps on a single web server. -# If used with Cloudflare, set encryption mode in SSL/TLS setting to "Full" to enable CF-to-app encryption. -# -# Using an SSL proxy like this requires turning on config.assume_ssl and config.force_ssl in production.rb! -# -# Don't use this when deploying to multiple web servers (then you have to terminate SSL at your load balancer). -# -# proxy: -# ssl: true -# host: app.example.com +# How you connect to your server +ssh: + user: root # If you use a different username to SSH to your server, specify it here -# Where you keep your container images. -registry: - # Alternatives: hub.docker.com / registry.digitalocean.com / ghcr.io / ... - server: localhost:5555 +# Automatic SSL +proxy: + ssl: true # Set this to false if you *don't* want SSL + host: fizzy.example.com # Set your server name here to use automatic SSL - # Needed for authenticated registries. - # username: your-user - - # Always use an access token rather than real password when possible. - # password: - # - KAMAL_REGISTRY_PASSWORD - -# Inject ENV variables into containers (secrets come from .kamal/secrets). +# Your application configuration (secrets come from .kamal/secrets). env: secret: - - RAILS_MASTER_KEY + - SECRET_KEY_BASE + - VAPID_PUBLIC_KEY + - VAPID_PRIVATE_KEY + - SMTP_USERNAME + - SMTP_PASSWORD clear: - # Run the Solid Queue Supervisor inside the web server's Puma process to do jobs. - # When you start using multiple servers, you should split out job processing to a dedicated machine. - SOLID_QUEUE_IN_PUMA: true + MAILER_FROM_ADDRESS: support@example.com # The email address that Fizzy sends email from + SOLID_QUEUE_IN_PUMA: true # Run background jobs in the app container - # Set number of processes dedicated to Solid Queue (default: 1) - # JOB_CONCURRENCY: 3 - # Set number of cores available to the application on each server (default: 1). - # WEB_CONCURRENCY: 2 +#-- General configuration --# - # Match this to any external database server to configure Active Record correctly - # Use fizzy-db for a db accessory server on same machine via local kamal docker network. - # DB_HOST: 192.168.0.2 +# Use a local registry to deploy +registry: + server: localhost:5555 - # Log everything from Rails - # RAILS_LOG_LEVEL: debug - -# Aliases are triggered with "bin/kamal ". You can overwrite arguments on invocation: -# "bin/kamal logs -r job" will tail logs from the first server in the job section. +# Handy aliases for interacting with your deployment. For eaxmple: `bin/kamal console` will connect to a +# Rails console in production. aliases: console: app exec --interactive --reuse "bin/rails console" shell: app exec --interactive --reuse "bash" @@ -76,45 +56,6 @@ volumes: # version inside the asset_path. asset_path: /rails/public/assets - # Configure the image builder. builder: arch: amd64 - - # # Build image via remote server (useful for faster amd64 builds on arm64 computers) - # remote: ssh://docker@docker-builder-server - # - # # Pass arguments and secrets to the Docker build process - # args: - # RUBY_VERSION: ruby-3.4.7 - # secrets: - # - GITHUB_TOKEN - # - RAILS_MASTER_KEY - -# Use a different ssh user than root -# ssh: -# user: app - -# Use accessory services (secrets come from .kamal/secrets). -# accessories: -# db: -# image: mysql:8.0 -# host: 192.168.0.2 -# # Change to 3306 to expose port to the world instead of just local network. -# port: "127.0.0.1:3306:3306" -# env: -# clear: -# MYSQL_ROOT_HOST: '%' -# secret: -# - MYSQL_ROOT_PASSWORD -# files: -# - config/mysql/production.cnf:/etc/mysql/my.cnf -# - db/production.sql:/docker-entrypoint-initdb.d/setup.sql -# directories: -# - data:/var/lib/mysql -# redis: -# image: valkey/valkey:8 -# host: 192.168.0.2 -# port: 6379 -# directories: -# - data:/data diff --git a/config/environments/production.rb b/config/environments/production.rb index bdb274efb..7b29a00d1 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -3,6 +3,21 @@ require "active_support/core_ext/integer/time" Rails.application.configure do # Settings specified here will take precedence over those in config/application.rb. + # Email provider Settings + # + # Configure these according to whichever email provider you use. An example setup + # using SMTP looks like the following: + # + # config.action_mailer.smtp_settings = { + # address: 'smtp.example.com', # The address of your email provider's SMTP server + # port: 2525, + # domain: 'example.com', # Your domain, which Fizzy will send email from + # user_name: ENV["SMTP_USERNAME"], + # password: ENV["SMTP_PASSWORD"], + # authentication: :plain, + # enable_starttls_auto: true + # } + # Code is not reloaded between requests. config.enable_reloading = false From 2cf718835b715190f78016eba0c0c6612854830f Mon Sep 17 00:00:00 2001 From: Andy Smith Date: Fri, 5 Dec 2025 11:16:11 -0600 Subject: [PATCH 13/70] Style updates for latest Lexxy version Style nested lists, highlight dropdown, and link dropdown. Also organizes the CSS file a bit better. --- app/assets/stylesheets/lexxy.css | 331 +++++++++---------- app/assets/stylesheets/rich-text-content.css | 14 +- 2 files changed, 172 insertions(+), 173 deletions(-) diff --git a/app/assets/stylesheets/lexxy.css b/app/assets/stylesheets/lexxy.css index 939a3fb5e..7b647caca 100644 --- a/app/assets/stylesheets/lexxy.css +++ b/app/assets/stylesheets/lexxy.css @@ -1,61 +1,39 @@ @layer components { + /* Editor + /* ------------------------------------------------------------------------ */ + lexxy-editor { display: block; position: relative; overflow: visible; figure.node--selected { - &:not(:has(img)) { - outline: var(--focus-ring-size) solid var(--focus-ring-color); - outline-offset: var(--focus-ring-offset); - } - &:has(img) { img { outline: var(--focus-ring-size) solid var(--focus-ring-color); outline-offset: var(--focus-ring-offset); } } - } - /* Lexical still uses the `lexxy-editor--empty` class if you have a list - started with no other characters. Here, we won't show the placeholder if - you've clicked the List button in the toolbar. */ - &.lexxy-editor--empty { - .lexxy-editor__content:not(:has(ul, ol))::before { - content: attr(placeholder); - color: currentColor; - cursor: text; - opacity: 0.66; - pointer-events: none; - position: absolute; - white-space: pre-line; + &:not(:has(img)) { + outline: var(--focus-ring-size) solid var(--focus-ring-color); + outline-offset: var(--focus-ring-offset); } } } - .lexxy-dialog-actions { - display: flex; - font-size: var(--text-x-small); - flex: 1 1 0%; - gap: var(--inline-space-half); - margin-block-start: var(--block-space-half); - - .btn { - --radius: 0.3em; - - inline-size: 100%; - justify-content: center; - - &:is([type="submit"]) { - --btn-background: var(--card-color, var(--color-link)); - --btn-color: var(--color-ink-inverted); - --focus-ring-color: var(--card-color, var(--color-link)); - } - } - - span { - inline-size: 100%; + /* Lexical uses the `lexxy-editor--empty` class even if you have a list + * started but haven't added other characters. Here, we hide the placeholder + * when you click the List button in the toolbar. */ + .lexxy-editor--empty { + .lexxy-editor__content:not(:has(ul, ol))::before { + content: attr(placeholder); + color: currentColor; + cursor: text; + opacity: 0.66; + pointer-events: none; + position: absolute; + white-space: pre-line; } } @@ -64,14 +42,9 @@ min-block-size: calc(7lh + var(--block-space)); outline: 0; - * { - &:first-child { - margin-block-start: 0; - } - - &:last-child { - margin-block-end: 0; - } + /* Allow color highlights to show through a bit */ + ::selection { + background: oklch(var(--lch-blue-light) / 0.5); } } @@ -81,6 +54,40 @@ outline: 2px dashed var(--color-selected-dark); } + .lexxy-code-language-picker { + -webkit-appearance: none; + appearance: none; + background-color: var(--color-canvas); + background-image: url("data:image/svg+xml,%3Csvg viewBox='0 0 24 24' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='m12 19.5c-.7 0-1.3-.3-1.7-.8l-9.8-11.1c-.7-.8-.6-1.9.2-2.6.8-.6 1.9-.6 2.5.2l8.6 9.8c0 .1.2.1.4 0l8.6-9.8c.7-.8 1.8-.9 2.6-.2s.9 1.8.2 2.6l-9.8 11.1c-.4.5-1.1.8-1.7.8z' fill='%23000'/%3E%3C/svg%3E"); + background-position: center right 0.9em; + background-repeat: no-repeat; + background-size: 0.5em; + border: 1px solid var(--color-ink-light); + border-radius: 99rem; + color: var(--color-ink); + cursor: pointer; + font-family: var(--font-base); + font-size: var(--text-x-small); + font-weight: 500; + inset-inline-end: 0; + line-height: 1.15lh; + margin: 0.75ch 0.75ch 0 0; + padding-inline: 1.5ch 1.8em; + text-align: start; + + @media (prefers-color-scheme: dark) { + background-image: url("data:image/svg+xml,%3Csvg viewBox='0 0 24 24' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='m12 19.5c-.7 0-1.3-.3-1.7-.8l-9.8-11.1c-.7-.8-.6-1.9.2-2.6.8-.6 1.9-.6 2.5.2l8.6 9.8c0 .1.2.1.4 0l8.6-9.8c.7-.8 1.8-.9 2.6-.2s.9 1.8.2 2.6l-9.8 11.1c-.4.5-1.1.8-1.7.8z' fill='%23fff'/%3E%3C/svg%3E"); + } + + option { + background-color: var(--color-canvas); + color: var(--color-ink); + } + } + + /* Toolbar + /* ------------------------------------------------------------------------ */ + lexxy-toolbar { --lexxy-toolbar-icon-size: 1em; @@ -96,21 +103,6 @@ position: sticky; inset-block-start: 0; z-index: 1; - - dialog { - background-color: var(--color-canvas); - border: 1px solid var(--color-ink-lighter); - border-radius: 0.5em; - box-shadow: var(--shadow); - color: var(--color-ink); - padding: var(--block-space) calc(var(--inline-space) * 1.5); - position: absolute; - z-index: 1; - - .input[type="url"] { - min-inline-size: 30ch; - } - } } .lexxy-editor__toolbar-button { @@ -152,11 +144,6 @@ } } - /* Temporarily hide the highlighter button until we tackle the styles */ - [name="strikethrough"] + .lexxy-editor__toolbar-dropdown { - display: none; - } - .lexxy-editor__toolbar-overflow-menu { background-color: var(--color-canvas); border-radius: 0.5ch; @@ -168,122 +155,122 @@ z-index: 1; } - :where(.lexxy-editor__toolbar-spacer) { + .lexxy-editor__toolbar-spacer { flex: 1; } - :where(.lexxy-highlight-dialog) { - .lexxy-highlight-dialog-content { - display: flex; - flex-direction: column; - gap: 0.75ch; + /* Dropdowns + /* ------------------------------------------------------------------------ */ - [data-button-group] { - display: flex; - flex-direction: row; - flex-wrap: wrap; - gap: 0.5ch; - } + .lexxy-editor__toolbar-dropdown-content { + --lexxy-dropdown-padding: 0.75rem; + --lexxy-dropdown-btn-size: 2rem; - button { - appearance: none; - background: var(--color-canvas); - block-size: 3.5ch; - border: none; - border-radius: 0.5ch; - color: var(--color-ink); - cursor: pointer; - display: grid; - font-size: inherit; - inline-size: 3.5ch; - place-items: center; - position: relative; - outline: none; - - &:after { - align-self: center; - content: "Aa"; - display: inline-block; - font-family: system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; - position: absolute; - inset-block-start: 0; - inset-block-end: 0; - inset-inline-end: 0; - inset-inline-start: 0; - } - - &:hover { - opacity: 0.8; - } - - &[aria-pressed="true"] { - box-shadow: 0 0 0 1px var(--color-canvas), 0 0 0 3px var(--color-link); - - &:after { - content: "✓"; - } - } - } - - .lexxy-highlight-dialog-reset { - inline-size: 100%; - - &[disabled] { - display: none; - } - - &:after { display: none; } - } - } - } - - .attachment__caption { - textarea { - background-color: var(--input-background, transparent); - border-radius: var(--input-border-radius, 0.5em); - border: var(--input-border-size, 1px) solid var(--input-border-color, var(--color-ink-medium)); - color: var(--input-color, var(--color-ink)); - font-size: max(16px, 1em); - inline-size: 100%; - line-height: inherit; - max-inline-size: 100%; - padding: var(--input-padding, 0.5em 0.8em); - resize: none; - } - } - - /* Based on .input, .input--select */ - .lexxy-code-language-picker { - -webkit-appearance: none; - appearance: none; background-color: var(--color-canvas); - background-image: url("data:image/svg+xml,%3Csvg viewBox='0 0 24 24' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='m12 19.5c-.7 0-1.3-.3-1.7-.8l-9.8-11.1c-.7-.8-.6-1.9.2-2.6.8-.6 1.9-.6 2.5.2l8.6 9.8c0 .1.2.1.4 0l8.6-9.8c.7-.8 1.8-.9 2.6-.2s.9 1.8.2 2.6l-9.8 11.1c-.4.5-1.1.8-1.7.8z' fill='%23000'/%3E%3C/svg%3E"); - background-position: center right 0.9em; - background-repeat: no-repeat; - background-size: 0.5em; border: 1px solid var(--color-ink-lighter); - border-radius: 0.3em; + border-radius: 0.5em; + box-shadow: var(--shadow); color: var(--color-ink); - font-family: var(--font-base); - font-size: var(--text-x-small); - font-weight: 500; - inset-inline-end: 0; - line-height: inherit; - margin: 0.3em 0.3em 0 0; - padding: 0.5em 1.8em 0.5em 1.2em; - text-align: start; + font-size: var(--text-small); + padding: var(--lexxy-dropdown-padding); + position: absolute; + z-index: 1; - @media (prefers-color-scheme: dark) { - background-image: url("data:image/svg+xml,%3Csvg viewBox='0 0 24 24' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='m12 19.5c-.7 0-1.3-.3-1.7-.8l-9.8-11.1c-.7-.8-.6-1.9.2-2.6.8-.6 1.9-.6 2.5.2l8.6 9.8c0 .1.2.1.4 0l8.6-9.8c.7-.8 1.8-.9 2.6-.2s.9 1.8.2 2.6l-9.8 11.1c-.4.5-1.1.8-1.7.8z' fill='%23fff'/%3E%3C/svg%3E"); - } - - option { - background-color: var(--color-canvas); - color: var(--color-ink); + button { + block-size: var(--lexxy-dropdown-btn-size); } } - /* Prompt + .lexxy-editor__toolbar-dropdown-actions { + display: flex; + font-size: var(--text-x-small); + flex: 1 1 0%; + gap: var(--lexxy-dropdown-padding); + margin-block-start: var(--lexxy-dropdown-padding); + + .btn { + --radius: 99rem; + + inline-size: 100%; + } + + .btn[type="submit"] { + --btn-background: var(--card-color, var(--color-link)); + --btn-color: var(--color-ink-inverted); + --focus-ring-color: var(--card-color, var(--color-link)); + } + + span { + inline-size: 100%; + } + } + + lexxy-link-dropdown { + .input { + min-inline-size: 30ch; + } + } + + lexxy-highlight-dropdown { + --gap: 0.5ch; + + [data-button-group] { + display: flex; + flex-direction: row; + flex-wrap: wrap; + gap: var(--gap); + + + & { + margin-block-start: var(--gap); + } + } + } + + .lexxy-highlight-button { + --outline-color: oklch(var(--lch-ink-darkest) / 0.2); + + appearance: none; + background: var(--color-canvas); + border: none; + border-radius: 0.5ch; + display: grid; + font-weight: 500; + inline-size: var(--lexxy-dropdown-btn-size); + place-items: center; + position: relative; + outline: none; + + &:after { + content: "Aa"; + } + + &:hover, + &[aria-pressed="true"] { + box-shadow: 0 0 0 1px var(--color-canvas), 0 0 0 3px var(--outline-color); + } + + &[aria-pressed="true"] { + --outline-color: var(--color-link); + + &:after { + content: "✓"; + } + } + } + + .lexxy-editor__toolbar-dropdown-reset { + background: var(--color-canvas); + border: 1px solid var(--color-ink-light); + border-radius: 99rem; + inline-size: 100%; + margin-block-start: var(--lexxy-dropdown-padding); + + &[disabled] { + display: none; + } + } + + /* Prompt ,enu (@mentions, etc.) /* ------------------------------------------------------------------------ */ .lexxy-prompt-menu { diff --git a/app/assets/stylesheets/rich-text-content.css b/app/assets/stylesheets/rich-text-content.css index ca3715cba..fdb8224fc 100644 --- a/app/assets/stylesheets/rich-text-content.css +++ b/app/assets/stylesheets/rich-text-content.css @@ -28,6 +28,14 @@ } } + :where(ol, ul) { + padding-inline-start: 3ch; + } + + :where(li:has(li)) { + list-style: none; + } + :where(b, strong, .lexxy-content__bold) { font-weight: 700; } @@ -210,9 +218,12 @@ --input-border-size: 0; --input-padding: 0; + background-color: var(--input-background, transparent); + border: none; color: inherit; inline-size: 100%; max-inline-size: 100%; + resize: none; text-align: center; &:focus { @@ -278,7 +289,8 @@ display: flex; flex-wrap: wrap; gap: 1ch; - inline-size: auto; + inline-size: 100%; + margin-inline: 0; .attachment__caption { display: grid; From 3acc58ff41e8a97cf1c6c2cf0049f243c16f5ef7 Mon Sep 17 00:00:00 2001 From: astrid Date: Fri, 5 Dec 2025 19:26:54 +0100 Subject: [PATCH 14/70] Add WEB_CONCURRENCY env support for Puma worker count --- config/puma.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/puma.rb b/config/puma.rb index 8d09b00ac..bc0e59587 100644 --- a/config/puma.rb +++ b/config/puma.rb @@ -25,7 +25,7 @@ if !Rails.env.local? # worker per CPU, 1 thread per worker and tune it from there. # # https://edgeguides.rubyonrails.org/tuning_performance_for_deployment.html#puma - workers Concurrent.physical_processor_count + workers ENV.fetch("WEB_CONCURRENCY", Concurrent.physical_processor_count).to_i threads 1, 1 # Tell the Ruby VM that we're finished booting up. From 430a0c612b0fbe231147b5c42a002a5d86fdc8bf Mon Sep 17 00:00:00 2001 From: ari Date: Fri, 5 Dec 2025 19:57:55 +0100 Subject: [PATCH 15/70] Update config/puma.rb (commit suggestion) Co-authored-by: Jeremy Daer --- config/puma.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/puma.rb b/config/puma.rb index bc0e59587..489ae0779 100644 --- a/config/puma.rb +++ b/config/puma.rb @@ -25,7 +25,7 @@ if !Rails.env.local? # worker per CPU, 1 thread per worker and tune it from there. # # https://edgeguides.rubyonrails.org/tuning_performance_for_deployment.html#puma - workers ENV.fetch("WEB_CONCURRENCY", Concurrent.physical_processor_count).to_i + workers Integer(ENV.fetch("WEB_CONCURRENCY") { Concurrent.physical_processor_count }) threads 1, 1 # Tell the Ruby VM that we're finished booting up. From cd3751f4c187607addf49e13e1d575d21a836d8f Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Fri, 5 Dec 2025 20:09:39 +0100 Subject: [PATCH 16/70] Fix Resolv::DNS always returning no results --- app/models/ssrf_protection.rb | 9 +++++++-- app/models/webhook/delivery.rb | 2 +- config/initializers/inflections.rb | 1 + 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/app/models/ssrf_protection.rb b/app/models/ssrf_protection.rb index f3df2511b..257debd87 100644 --- a/app/models/ssrf_protection.rb +++ b/app/models/ssrf_protection.rb @@ -1,8 +1,13 @@ -module SsrfProtection +module SSRFProtection extend self DNS_RESOLUTION_TIMEOUT = 2 + DNS_NAMESERVERS = %w[ + 1.1.1.1 + 8.8.8.8 + ] + DISALLOWED_IP_RANGES = [ IPAddr.new("0.0.0.0/8") # Broadcasts ].freeze @@ -22,7 +27,7 @@ module SsrfProtection def resolve_dns(hostname) ip_addresses = [] - Resolv::DNS.open(timeouts: DNS_RESOLUTION_TIMEOUT) do |dns| + Resolv::DNS.open(nameserver: DNS_NAMESERVERS, timeouts: DNS_RESOLUTION_TIMEOUT) do |dns| dns.each_address(hostname) do |ip_address| ip_addresses << IPAddr.new(ip_address.to_s) end diff --git a/app/models/webhook/delivery.rb b/app/models/webhook/delivery.rb index b635baac5..daf540106 100644 --- a/app/models/webhook/delivery.rb +++ b/app/models/webhook/delivery.rb @@ -70,7 +70,7 @@ class Webhook::Delivery < ApplicationRecord def resolved_ip return @resolved_ip if defined?(@resolved_ip) - @resolved_ip = SsrfProtection.resolve_public_ip(uri.host) + @resolved_ip = SSRFProtection.resolve_public_ip(uri.host) end def uri diff --git a/config/initializers/inflections.rb b/config/initializers/inflections.rb index 62ffe294f..7e49acd7e 100644 --- a/config/initializers/inflections.rb +++ b/config/initializers/inflections.rb @@ -13,6 +13,7 @@ # These inflection rules are supported but not enabled by default: ActiveSupport::Inflector.inflections(:en) do |inflect| inflect.acronym "SQLite" + inflect.acronym "SSRF" inflect.singular "quotas", "quota" inflect.plural "quota", "quotas" From fcce276dc912bf4dca482af9216fdea13fa13c84 Mon Sep 17 00:00:00 2001 From: astrid Date: Fri, 5 Dec 2025 20:20:13 +0100 Subject: [PATCH 17/70] Update JOB_CONCURRENCY to match upstream Rails --- config/queue.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/queue.yml b/config/queue.yml index 130b0d9bd..3f5615fa8 100644 --- a/config/queue.yml +++ b/config/queue.yml @@ -5,7 +5,7 @@ default: &default workers: - queues: [ "default", "solid_queue_recurring", "backend", "webhooks" ] threads: 3 - processes: <%= ENV.fetch("JOB_CONCURRENCY", Concurrent.physical_processor_count) %> + processes: <%= Integer(ENV.fetch("JOB_CONCURRENCY") { Concurrent.physical_processor_count }) %> polling_interval: 0.1 development: *default From 1b3d7d4276778b04d3581ba6ea94c5ddb3b51a19 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Fri, 5 Dec 2025 20:43:56 +0100 Subject: [PATCH 18/70] Fix ipaddr setter --- app/models/ssrf_protection.rb | 2 +- app/models/webhook/delivery.rb | 3 ++- test/models/webhook/delivery_test.rb | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/ssrf_protection.rb b/app/models/ssrf_protection.rb index 257debd87..e43455b39 100644 --- a/app/models/ssrf_protection.rb +++ b/app/models/ssrf_protection.rb @@ -15,7 +15,7 @@ module SSRFProtection def resolve_public_ip(hostname) ip_addresses = resolve_dns(hostname) public_ips = ip_addresses.reject { |ip| private_address?(ip) } - public_ips.first&.to_s + public_ips.sort_by { |ipaddr| ipaddr.ipv4? ? 0 : 1 }.first&.to_s end def private_address?(ip) diff --git a/app/models/webhook/delivery.rb b/app/models/webhook/delivery.rb index daf540106..79db20b90 100644 --- a/app/models/webhook/delivery.rb +++ b/app/models/webhook/delivery.rb @@ -78,7 +78,8 @@ class Webhook::Delivery < ApplicationRecord end def http - Net::HTTP.new(uri.host, uri.port, ipaddr: resolved_ip).tap do |http| + Net::HTTP.new(uri.host, uri.port).tap do |http| + http.ipaddr = resolved_ip http.use_ssl = (uri.scheme == "https") http.open_timeout = ENDPOINT_TIMEOUT http.read_timeout = ENDPOINT_TIMEOUT diff --git a/test/models/webhook/delivery_test.rb b/test/models/webhook/delivery_test.rb index c28666752..30f91390e 100644 --- a/test/models/webhook/delivery_test.rb +++ b/test/models/webhook/delivery_test.rb @@ -298,11 +298,12 @@ class Webhook::DeliveryTest < ActiveSupport::TestCase # Verify Net::HTTP.new is called with the pinned IP http_mock = mock("http") http_mock.stubs(:use_ssl=) + http_mock.stubs(:ipaddr=) http_mock.stubs(:open_timeout=) http_mock.stubs(:read_timeout=) http_mock.stubs(:request).returns(stub(code: "200")) - Net::HTTP.expects(:new).with("example.com", 443, ipaddr: PUBLIC_TEST_IP).returns(http_mock) + Net::HTTP.expects(:new).with("example.com", 443).returns(http_mock) delivery.deliver From 330be4ec72d956d56a68baec7f16058024877ab3 Mon Sep 17 00:00:00 2001 From: Justin Starner Date: Fri, 5 Dec 2025 15:25:24 -0500 Subject: [PATCH 19/70] Position modal buttons using justify-center. --- app/views/boards/edit/_delete.html.erb | 2 +- app/views/cards/_delete.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/boards/edit/_delete.html.erb b/app/views/boards/edit/_delete.html.erb index 47a7a3ed1..ec346f590 100644 --- a/app/views/boards/edit/_delete.html.erb +++ b/app/views/boards/edit/_delete.html.erb @@ -6,7 +6,7 @@

Delete this board?

Are you sure you want to permanently delete this board and all the cards on it? This can't be undone.

-
+
<%= button_to board_path(board), method: :delete, class: "btn txt-negative", data: { turbo_frame: "_top" } do %> Delete board diff --git a/app/views/cards/_delete.html.erb b/app/views/cards/_delete.html.erb index 5e6279478..43afbbad2 100644 --- a/app/views/cards/_delete.html.erb +++ b/app/views/cards/_delete.html.erb @@ -6,7 +6,7 @@

Delete this card?

Are you sure you want to permanently delete this card?

-
+
<%= button_to card_path(card), method: :delete, class: "btn txt-negative", data: { turbo_frame: "_top" } do %> Delete card From 6006491ab44897ce6df8428950cf0b342ef9fb38 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 5 Dec 2025 14:57:50 -0800 Subject: [PATCH 20/70] Content Security Policy (#1964) Default to a very narrow policy since there are no CDNs or third-party resources to contend with. Configurable via: - config.x.content_security_policy.* for fizzy-saas gem overrides - DISABLE_CSP to skip entirely - CSP_REPORT_ONLY to enable report-only mode - CSP_REPORT_URI for violation reporting --- Gemfile.saas.lock | 2 +- .../initializers/content_security_policy.rb | 53 +++++++++++-------- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index 5cd308155..b6ffda1d4 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -21,7 +21,7 @@ GIT GIT remote: https://github.com/basecamp/fizzy-saas - revision: 97d126e0a6084905aceaadf4facaa2a48b44e124 + revision: 3b30f3d56d8318bb9256469cb91c06a5a5c760a6 specs: fizzy-saas (0.1.0) audits1984 diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index b3076b38f..94a8bd41a 100644 --- a/config/initializers/content_security_policy.rb +++ b/config/initializers/content_security_policy.rb @@ -1,25 +1,34 @@ # Be sure to restart your server when you modify this file. -# Define an application-wide content security policy. -# See the Securing Rails Applications Guide for more information: -# https://guides.rubyonrails.org/security.html#content-security-policy-header +# Define an application-wide Content Security Policy. +# https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy -# Rails.application.configure do -# config.content_security_policy do |policy| -# policy.default_src :self, :https -# policy.font_src :self, :https, :data -# policy.img_src :self, :https, :data -# policy.object_src :none -# policy.script_src :self, :https -# policy.style_src :self, :https -# # Specify URI for violation reports -# # policy.report_uri "/csp-violation-report-endpoint" -# end -# -# # Generate session nonces for permitted importmap, inline scripts, and inline styles. -# config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s } -# config.content_security_policy_nonce_directives = %w(script-src style-src) -# -# # Report violations without enforcing the policy. -# # config.content_security_policy_report_only = true -# end +Rails.application.configure do + # Configure from environment variables (fizzy-saas can override by setting config.x values first) + config.x.content_security_policy.report_uri ||= ENV["CSP_REPORT_URI"] + config.x.content_security_policy.report_only ||= ENV["CSP_REPORT_ONLY"] == "true" + + # Generate nonces for importmap and inline scripts + config.content_security_policy_nonce_generator = ->(request) { SecureRandom.base64(16) } + config.content_security_policy_nonce_directives = %w[ script-src ] + + config.content_security_policy do |policy| + policy.script_src :self + policy.style_src :self, :unsafe_inline + policy.img_src :self, "blob:", "data:", "https:" + policy.font_src :self + policy.object_src :none + + policy.base_uri :none + policy.form_action :self + policy.frame_ancestors :self + + # Specify URI for violation reports (e.g., Sentry CSP endpoint) + if report_uri = config.x.content_security_policy.report_uri + policy.report_uri report_uri + end + end + + # Report violations without enforcing the policy. + config.content_security_policy_report_only = config.x.content_security_policy.report_only +end unless ENV["DISABLE_CSP"] From 1b4fb0db5f142e21cfad41c8ceba51b849ffbd13 Mon Sep 17 00:00:00 2001 From: astrid Date: Sat, 6 Dec 2025 00:09:59 +0100 Subject: [PATCH 21/70] Update README.md for more consistent codeblocks --- README.md | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 613e17ecc..e816c43d2 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,6 @@ This is the source code of [Fizzy](https://fizzy.do/), the Kanban tracking tool for issues and ideas by [37signals](https://37signals.com). - ## Deploying Fizzy If you'd like to run Fizzy on your own server, we recommend deploying it with [Kamal](https://kamal-deploy.org/). @@ -48,7 +47,7 @@ Refer to the [Kamal documentation](https://kamal-deploy.org/docs/configuration/e To store your secrets, create the file `.kamal/secrets` and enter something like the following: -``` +```ini SECRET_KEY_BASE=12345 VAPID_PUBLIC_KEY=something VAPID_PRIVATE_KEY=somethingelse @@ -61,16 +60,18 @@ The values you enter here will be specific to you, and you can get or create the - `SECRET_KEY_BASE` should be a long, random secret. You can run `bin/rails secret` to create a suitable value for this. - `VAPID_PUBLIC_KEY` & `VAPID_PRIVATE_KEY` are a pair of credentials that are used for sending notifications. You can create your own keys by starting a development console with: - bin/rails c +```sh +bin/rails c +``` - And then run the following to create a new pair of keys: +And then run the following to create a new pair of keys: - ```ruby - vapid_key = WebPush.generate_key +```ruby +vapid_key = WebPush.generate_key - puts "VAPID_PRIVATE_KEY=#{vapid_key.private_key}" - puts "VAPID_PUBLIC_KEY=#{vapid_key.public_key}" - ``` +puts "VAPID_PRIVATE_KEY=#{vapid_key.private_key}" +puts "VAPID_PUBLIC_KEY=#{vapid_key.public_key}" +``` - `SMTP_USERNAME`/`SMTP_PASSWORD` are credentials you should get from your email provider. @@ -82,14 +83,17 @@ Once you've made all those changes, commit them to your fork so they're saved. You can now do your first deploy by running: - bin/kamal setup +```sh +bin/kamal setup +``` This will set up Docker (if needed), build your Fizzy app container, configure it, and start it running. After the first deploy is done, any subsequent steps won't need to do that initial setup. So for future deploys you can just run: - bin/kamal deploy - +```sh +bin/kamal deploy +``` ## Development @@ -132,11 +136,15 @@ puts "VAPID_PUBLIC_KEY=#{vapid_key.public_key}" For fast feedback loops, unit tests can be run with: - bin/rails test +```sh +bin/rails test +``` The full continuous integration tests can be run with: - bin/ci +```sh +bin/ci +``` ### Database configuration @@ -155,24 +163,22 @@ You can view email previews at http://fizzy.localhost:3006/rails/mailers. You can enable or disable [`letter_opener`](https://github.com/ryanb/letter_opener) to open sent emails automatically with: - bin/rails dev:email +```sh +bin/rails dev:email +``` Under the hood, this will create or remove `tmp/email-dev.txt`. - ## SaaS gem 37signals bundles Fizzy with [`fizzy-saas`](https://github.com/basecamp/fizzy-saas), a companion gem that links Fizzy with our billing system and contains our production setup. This gem depends on some private git repositories and it is not meant to be used by third parties. But we hope it can serve as inspiration for anyone wanting to run fizzy on their own infrastructure. - ## Contributing We welcome contributions! Please read our [style guide](STYLE.md) before submitting code. - ## License Fizzy is released under the [O'Saasy License](LICENSE.md). - From c2935cfa607cdc6ae50b74a2f22a6ef295ec8e84 Mon Sep 17 00:00:00 2001 From: astrid Date: Sat, 6 Dec 2025 00:22:54 +0100 Subject: [PATCH 22/70] Readd padding to VAPID_PUBLIC_KEY/VAPID_PRIVATE_KEY --- README.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index e816c43d2..07f55614e 100644 --- a/README.md +++ b/README.md @@ -60,18 +60,18 @@ The values you enter here will be specific to you, and you can get or create the - `SECRET_KEY_BASE` should be a long, random secret. You can run `bin/rails secret` to create a suitable value for this. - `VAPID_PUBLIC_KEY` & `VAPID_PRIVATE_KEY` are a pair of credentials that are used for sending notifications. You can create your own keys by starting a development console with: -```sh -bin/rails c -``` + ```sh + bin/rails c + ``` -And then run the following to create a new pair of keys: + And then run the following to create a new pair of keys: -```ruby -vapid_key = WebPush.generate_key + ```ruby + vapid_key = WebPush.generate_key -puts "VAPID_PRIVATE_KEY=#{vapid_key.private_key}" -puts "VAPID_PUBLIC_KEY=#{vapid_key.public_key}" -``` + puts "VAPID_PRIVATE_KEY=#{vapid_key.private_key}" + puts "VAPID_PUBLIC_KEY=#{vapid_key.public_key}" + ``` - `SMTP_USERNAME`/`SMTP_PASSWORD` are credentials you should get from your email provider. From db55923fdb64848dfe8a85049f4cd705ada7908f Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 5 Dec 2025 16:18:01 -0800 Subject: [PATCH 23/70] Fix lightbox transitionend listener leak Switch to a Stimulus action for automatic handling --- app/javascript/controllers/lightbox_controller.js | 8 -------- app/views/layouts/_lightbox.html.erb | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/app/javascript/controllers/lightbox_controller.js b/app/javascript/controllers/lightbox_controller.js index b39566b44..e8bc48f03 100644 --- a/app/javascript/controllers/lightbox_controller.js +++ b/app/javascript/controllers/lightbox_controller.js @@ -3,14 +3,6 @@ import { Controller } from "@hotwired/stimulus" export default class extends Controller { static targets = [ "caption", "image", "dialog", "zoomedImage" ] - connect() { - this.dialogTarget.addEventListener('transitionend', this.handleTransitionEnd.bind(this)) - } - - disconnect() { - this.dialogTarget.removeEventListener('transitionend', this.handleTransitionEnd.bind(this)) - } - open(event) { this.dialogTarget.showModal() this.#set(event.target.closest("a")) diff --git a/app/views/layouts/_lightbox.html.erb b/app/views/layouts/_lightbox.html.erb index 9f0e23ebf..74f5597a9 100644 --- a/app/views/layouts/_lightbox.html.erb +++ b/app/views/layouts/_lightbox.html.erb @@ -1,5 +1,5 @@ <%= tag.dialog class:"lightbox", aria: { label: "Image Viewer (Press escape to close)" }, - data: { controller: "dialog", dialog_target: "dialog", dialog_modal_value: "true", lightbox_target: "dialog", action: "keydown.esc->dialog#close:stop" } do %> + data: { controller: "dialog", dialog_target: "dialog", dialog_modal_value: "true", lightbox_target: "dialog", action: "keydown.esc->dialog#close:stop transitionend->lightbox#handleTransitionEnd" } do %>
From 345f457513963d13998450dab1ed2077d505e5a1 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 5 Dec 2025 16:57:03 -0800 Subject: [PATCH 24/70] Tune CSP for user extensions and close implicit-allow gaps (#1974) Loosen font-src and add media-src to allow browser extensions like accessibility fonts (OpenDyslexic) and user style managers to work without CSP violations. Add default-src and connect-src directives to close security gaps where unspecified directives were implicitly allowing all sources. --- config/initializers/content_security_policy.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index 94a8bd41a..fc63e9479 100644 --- a/config/initializers/content_security_policy.rb +++ b/config/initializers/content_security_policy.rb @@ -13,10 +13,13 @@ Rails.application.configure do config.content_security_policy_nonce_directives = %w[ script-src ] config.content_security_policy do |policy| + policy.default_src :self policy.script_src :self policy.style_src :self, :unsafe_inline + policy.connect_src :self policy.img_src :self, "blob:", "data:", "https:" - policy.font_src :self + policy.font_src :self, "data:", "https:" + policy.media_src :self, "blob:", "data:", "https:" policy.object_src :none policy.base_uri :none From fa549a370b9ed6fc35c109750367bf7ae97c744f Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 5 Dec 2025 16:16:58 -0500 Subject: [PATCH 25/70] Add a system test for joining an account Reworked the magic link stimulus controller, because the system test was causing double-submission of the form (because the event was bubbling up). I think that change simplifies the form and will still work well for iOS devices. --- .../controllers/magic_link_controller.js | 18 +++++++++++------ app/views/sessions/magic_links/show.html.erb | 2 +- test/system/smoke_test.rb | 20 +++++++++++++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/app/javascript/controllers/magic_link_controller.js b/app/javascript/controllers/magic_link_controller.js index 4d6c45ab5..3e2e4618c 100644 --- a/app/javascript/controllers/magic_link_controller.js +++ b/app/javascript/controllers/magic_link_controller.js @@ -4,12 +4,18 @@ import { onNextEventLoopTick } from "helpers/timing_helpers" export default class extends Controller { static targets = [ "input" ] + submitOnEnter(event) { + event.preventDefault() + this.submit() + } + + submitOnPaste() { + onNextEventLoopTick(() => this.submit()) + } + submit() { - onNextEventLoopTick(() => { - if (!this.inputTarget.disabled) { - this.element.submit() - this.inputTarget.disabled = true - } - }) + if (this.inputTarget.disabled) return + this.element.submit() + this.inputTarget.disabled = true } } diff --git a/app/views/sessions/magic_links/show.html.erb b/app/views/sessions/magic_links/show.html.erb index 931156ff6..434bb93c3 100644 --- a/app/views/sessions/magic_links/show.html.erb +++ b/app/views/sessions/magic_links/show.html.erb @@ -10,7 +10,7 @@ <%= form.text_field :code, required: true, class: "input center txt-align-enter txt-large", autofocus: true, autocorrect: "off", autocapitalize: "off", spellcheck: "false", "data-1p-ignore": true, autocomplete: "one-time-code", maxlength: "6", placeholder: "••••••", value: params[:code], - data: { magic_link_target: "input", action: "keydown.enter->magic-link#submit paste->magic-link#submit" } %> + data: { magic_link_target: "input", action: "keydown.enter->magic-link#submitOnEnter paste->magic-link#submitOnPaste" } %> <% end %>

The code you receive will work for <%= distance_of_time_in_words(MagicLink::EXPIRATION_TIME) %>.

diff --git a/test/system/smoke_test.rb b/test/system/smoke_test.rb index e4bc9cae9..c70d42e95 100644 --- a/test/system/smoke_test.rb +++ b/test/system/smoke_test.rb @@ -1,6 +1,26 @@ require "application_system_test_case" class SmokeTest < ApplicationSystemTestCase + test "joining an account" do + account = accounts("37s") + + visit join_url(code: account.join_code.code, script_name: account.slug) + fill_in "Email address", with: "newbie@example.com" + click_on "Continue" + + assert_selector "h1", text: "Check your email" + identity = Identity.find_by!(email_address: "newbie@example.com") + code = identity.magic_links.active.first.code + fill_in "code", with: code + send_keys :enter + + assert_selector "input[id=user_name]" + fill_in "Full name", with: "New Bee" + click_on "Continue" + + assert_selector "h1", text: "Writebook" + end + test "create a card" do sign_in_as(users(:david)) From 4602cd3cdd87949aafd151b23059dd70b4457292 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 4 Dec 2025 20:20:20 -0500 Subject: [PATCH 26/70] Add verified_at timestamp to use for spam prevention User are marked as verified after a join code is redeemed. The user is redirected to Users::VerificationsController, either: - after submitting a valid magic link code, - or immediately after redeeming the join code (if they're already authenticated with the correct identity) Account owners are automatically verified when the account is created (because they have already provided a magic link code at that point). This sets up for later commits that will backfill existing users and require verification before sending notification emails. Co-Authored-By: Claude --- app/controllers/join_codes_controller.rb | 4 +-- .../users/verifications_controller.rb | 11 +++++++ app/models/account.rb | 2 +- app/models/user.rb | 8 +++++ app/views/users/verifications/new.html.erb | 1 + config/routes.rb | 1 + ...20251205010536_add_verified_at_to_users.rb | 5 +++ db/schema.rb | 3 +- db/schema_sqlite.rb | 3 +- db/seeds.rb | 2 +- .../controllers/join_codes_controller_test.rb | 6 ++-- .../users/verifications_controller_test.rb | 24 ++++++++++++++ test/fixtures/users.yml | 5 +++ test/models/account_test.rb | 2 ++ test/models/user_test.rb | 32 +++++++++++++++++++ test/system/smoke_test.rb | 1 + 16 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 app/controllers/users/verifications_controller.rb create mode 100644 app/views/users/verifications/new.html.erb create mode 100644 db/migrate/20251205010536_add_verified_at_to_users.rb create mode 100644 test/controllers/users/verifications_controller_test.rb diff --git a/app/controllers/join_codes_controller.rb b/app/controllers/join_codes_controller.rb index 971b3c6f8..eb328df18 100644 --- a/app/controllers/join_codes_controller.rb +++ b/app/controllers/join_codes_controller.rb @@ -18,7 +18,7 @@ class JoinCodesController < ApplicationController if identity == Current.identity && user.setup? redirect_to landing_url(script_name: @join_code.account.slug) elsif identity == Current.identity - redirect_to new_users_join_url(script_name: @join_code.account.slug) + redirect_to new_users_verification_url(script_name: @join_code.account.slug) else logout_and_send_new_magic_link(identity) redirect_to session_magic_link_url(script_name: nil) @@ -44,6 +44,6 @@ class JoinCodesController < ApplicationController magic_link = identity.send_magic_link serve_development_magic_link(magic_link) - session[:return_to_after_authenticating] = new_users_join_url(script_name: @join_code.account.slug) + session[:return_to_after_authenticating] = new_users_verification_url(script_name: @join_code.account.slug) end end diff --git a/app/controllers/users/verifications_controller.rb b/app/controllers/users/verifications_controller.rb new file mode 100644 index 000000000..d99492576 --- /dev/null +++ b/app/controllers/users/verifications_controller.rb @@ -0,0 +1,11 @@ +class Users::VerificationsController < ApplicationController + layout "public" + + def new + end + + def create + Current.user.verify + redirect_to new_users_join_path + end +end diff --git a/app/models/account.rb b/app/models/account.rb index 50f3bf3ee..61a636adb 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -21,7 +21,7 @@ class Account < ApplicationRecord def create_with_owner(account:, owner:) create!(**account).tap do |account| account.users.create!(role: :system, name: "System") - account.users.create!(**owner.reverse_merge(role: "owner")) + account.users.create!(**owner.reverse_merge(role: "owner", verified_at: Time.current)) end end end diff --git a/app/models/user.rb b/app/models/user.rb index 8c0ac42a5..20ee1c40c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -28,6 +28,14 @@ class User < ApplicationRecord name != identity.email_address end + def verified? + verified_at.present? + end + + def verify + update!(verified_at: Time.current) unless verified? + end + private def close_remote_connections ActionCable.server.remote_connections.where(current_user: self).disconnect(reconnect: false) diff --git a/app/views/users/verifications/new.html.erb b/app/views/users/verifications/new.html.erb new file mode 100644 index 000000000..fe830cd28 --- /dev/null +++ b/app/views/users/verifications/new.html.erb @@ -0,0 +1 @@ +<%= auto_submit_form_with url: users_verifications_path, method: :post %> diff --git a/config/routes.rb b/config/routes.rb index 9fa8f81bf..e67e4ef7c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -138,6 +138,7 @@ Rails.application.routes.draw do namespace :users do resources :joins + resources :verifications, only: %i[ new create ] end resource :session do diff --git a/db/migrate/20251205010536_add_verified_at_to_users.rb b/db/migrate/20251205010536_add_verified_at_to_users.rb new file mode 100644 index 000000000..b853cb952 --- /dev/null +++ b/db/migrate/20251205010536_add_verified_at_to_users.rb @@ -0,0 +1,5 @@ +class AddVerifiedAtToUsers < ActiveRecord::Migration[8.2] + def change + add_column :users, :verified_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index b84d3c160..42458db17 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_12_01_100607) do +ActiveRecord::Schema[8.2].define(version: 2025_12_05_010536) do create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -726,6 +726,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_12_01_100607) do t.string "name", null: false t.string "role", default: "member", null: false t.datetime "updated_at", null: false + t.datetime "verified_at" t.index ["account_id", "identity_id"], name: "index_users_on_account_id_and_identity_id", unique: true t.index ["account_id", "role"], name: "index_users_on_account_id_and_role" t.index ["identity_id"], name: "index_users_on_identity_id" diff --git a/db/schema_sqlite.rb b/db/schema_sqlite.rb index 6f6d6cf53..1dd2b3e00 100644 --- a/db/schema_sqlite.rb +++ b/db/schema_sqlite.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_12_01_100607) do +ActiveRecord::Schema[8.2].define(version: 2025_12_05_010536) do create_table "accesses", id: :uuid, force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -499,6 +499,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_12_01_100607) do t.string "name", limit: 255, null: false t.string "role", limit: 255, default: "member", null: false t.datetime "updated_at", null: false + t.datetime "verified_at" t.index ["account_id", "identity_id"], name: "index_users_on_account_id_and_identity_id", unique: true t.index ["account_id", "role"], name: "index_users_on_account_id_and_role" t.index ["identity_id"], name: "index_users_on_identity_id" diff --git a/db/seeds.rb b/db/seeds.rb index bb8db7db8..7cba1b5f9 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -36,7 +36,7 @@ else if user = identity.users.find_by(account: Current.account) user else - User.create!(name: full_name, identity: identity, account: Current.account) + User.create!(name: full_name, identity: identity, account: Current.account, verified_at: Time.current) end end diff --git a/test/controllers/join_codes_controller_test.rb b/test/controllers/join_codes_controller_test.rb index fe8c820d9..0a4bf5793 100644 --- a/test/controllers/join_codes_controller_test.rb +++ b/test/controllers/join_codes_controller_test.rb @@ -36,7 +36,7 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest end assert_redirected_to session_magic_link_url(script_name: nil) - assert_equal new_users_join_url(script_name: @account.slug), session[:return_to_after_authenticating] + assert_equal new_users_verification_url(script_name: @account.slug), session[:return_to_after_authenticating] end test "create for existing identity" do @@ -55,7 +55,7 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest assert_redirected_to landing_url(script_name: @account.slug) end - test "create for signed-in identity without a user in the account redirects to user setup" do + test "create for signed-in identity without a user in the account redirects to verification" do identity = identities(:mike) sign_in_as :mike @@ -67,6 +67,6 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest end end - assert_redirected_to new_users_join_url(script_name: @account.slug) + assert_redirected_to new_users_verification_url(script_name: @account.slug) end end diff --git a/test/controllers/users/verifications_controller_test.rb b/test/controllers/users/verifications_controller_test.rb new file mode 100644 index 000000000..25ecdba1c --- /dev/null +++ b/test/controllers/users/verifications_controller_test.rb @@ -0,0 +1,24 @@ +require "test_helper" + +class Users::VerificationsControllerTest < ActionDispatch::IntegrationTest + test "new renders the auto-submit form" do + sign_in_as :david + + get new_users_verification_path + + assert_response :ok + end + + test "create verifies the user and redirects to join" do + sign_in_as :david + + user = users(:david) + user.update_column(:verified_at, nil) + assert_not user.verified? + + post users_verifications_path + + assert_redirected_to new_users_join_path + assert user.reload.verified? + end +end diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 4c7dad414..cf4d9e6d0 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -4,6 +4,7 @@ david: role: member identity: david account: 37s_uuid + verified_at: <%= Time.current.to_fs(:db) %> jz: id: <%= ActiveRecord::FixtureSet.identify("jz", :uuid) %> @@ -11,6 +12,7 @@ jz: role: member identity: jz account: 37s_uuid + verified_at: <%= Time.current.to_fs(:db) %> jason: id: <%= ActiveRecord::FixtureSet.identify("jason", :uuid) %> @@ -18,6 +20,7 @@ jason: role: owner identity: jason account: 37s_uuid + verified_at: <%= Time.current.to_fs(:db) %> kevin: id: <%= ActiveRecord::FixtureSet.identify("kevin", :uuid) %> @@ -25,6 +28,7 @@ kevin: role: admin identity: kevin account: 37s_uuid + verified_at: <%= Time.current.to_fs(:db) %> system: id: <%= ActiveRecord::FixtureSet.identify("system", :uuid) %> @@ -38,6 +42,7 @@ mike: role: admin identity: mike account: initech_uuid + verified_at: <%= Time.current.to_fs(:db) %> system_initech: id: <%= ActiveRecord::FixtureSet.identify("system_initech", :uuid) %> diff --git a/test/models/account_test.rb b/test/models/account_test.rb index 490d6e32e..75da41fb9 100644 --- a/test/models/account_test.rb +++ b/test/models/account_test.rb @@ -44,6 +44,8 @@ class AccountTest < ActiveSupport::TestCase assert owner.admin?, "owner should also be considered an admin" assert_predicate account.system_user, :present? + + assert owner.verified?, "owner should be verified on account creation" end end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 36b0e7986..645e3a386 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -48,4 +48,36 @@ class UserTest < ActiveSupport::TestCase user.update!(name: "Kevin") assert user.setup? end + + test "verified? returns true when verified_at is present" do + user = users(:david) + user.update_column(:verified_at, Time.current) + + assert user.verified? + end + + test "verified? returns false when verified_at is nil" do + user = users(:david) + user.update_column(:verified_at, nil) + + assert_not user.verified? + end + + test "verify sets verified_at when not already verified" do + user = users(:david) + user.update_column(:verified_at, nil) + + assert_nil user.verified_at + user.verify + assert_not_nil user.reload.verified_at + end + + test "verify does not update verified_at when already verified" do + user = users(:david) + original_time = 1.day.ago + user.update_column(:verified_at, original_time) + + user.verify + assert_equal original_time.to_i, user.reload.verified_at.to_i + end end diff --git a/test/system/smoke_test.rb b/test/system/smoke_test.rb index c70d42e95..485fba810 100644 --- a/test/system/smoke_test.rb +++ b/test/system/smoke_test.rb @@ -15,6 +15,7 @@ class SmokeTest < ApplicationSystemTestCase send_keys :enter assert_selector "input[id=user_name]" + assert account.users.find_by!(identity:).verified?, "User was not properly verified" fill_in "Full name", with: "New Bee" click_on "Continue" From e174206233add17c8d1e1aa9efb03b70ad2a961a Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 5 Dec 2025 08:42:58 -0500 Subject: [PATCH 27/70] Add backfill script for verified_at timestamps Identifies users who have demonstrated authentication by looking for evidence of real activity: owners, content creators (cards, comments, boards, events), action takers (assigners, closers, postponers, reactors, pinners, filter creators), board accessors, export requesters, push subscribers, setup completers, and users with active sessions. Run this script after deploying the verified_at column to backfill existing users before enabling the notification guard. Co-Authored-By: Claude --- .../20251205-backfill-verified-at.rb | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100755 script/migrations/20251205-backfill-verified-at.rb diff --git a/script/migrations/20251205-backfill-verified-at.rb b/script/migrations/20251205-backfill-verified-at.rb new file mode 100755 index 000000000..7cd4f0030 --- /dev/null +++ b/script/migrations/20251205-backfill-verified-at.rb @@ -0,0 +1,112 @@ +#!/usr/bin/env ruby + +require_relative "../../config/environment" + +BACKFILL_TIMESTAMP = Time.parse("2025-12-02 12:00:00 UTC") + +def collect_verified_user_ids + verified_ids = Set.new + + # Owners (they created the account) + verified_ids.merge(User.where(role: :owner).pluck(:id)) + puts "After owners: #{verified_ids.size} users" + + # Card creators + verified_ids.merge(Card.distinct.pluck(:creator_id).compact) + puts "After card creators: #{verified_ids.size} users" + + # Comment creators + verified_ids.merge(Comment.distinct.pluck(:creator_id).compact) + puts "After comment creators: #{verified_ids.size} users" + + # Board creators + verified_ids.merge(Board.distinct.pluck(:creator_id).compact) + puts "After board creators: #{verified_ids.size} users" + + # Event creators + verified_ids.merge(Event.distinct.pluck(:creator_id).compact) + puts "After event creators: #{verified_ids.size} users" + + # Assigners (not assignees - they could be assigned without logging in) + verified_ids.merge(Assignment.distinct.pluck(:assigner_id).compact) + puts "After assigners: #{verified_ids.size} users" + + # Manual closers (user_id is nil for automatic closures) + verified_ids.merge(Closure.where.not(user_id: nil).distinct.pluck(:user_id).compact) + puts "After closers: #{verified_ids.size} users" + + # Manual postponers (user_id is nil for automatic entropy postponements) + verified_ids.merge(Card::NotNow.where.not(user_id: nil).distinct.pluck(:user_id).compact) + puts "After postponers: #{verified_ids.size} users" + + # Reactors + verified_ids.merge(Reaction.distinct.pluck(:reacter_id).compact) + puts "After reactors: #{verified_ids.size} users" + + # Filter creators + verified_ids.merge(Filter.distinct.pluck(:creator_id).compact) + puts "After filter creators: #{verified_ids.size} users" + + # Pinners + verified_ids.merge(Pin.distinct.pluck(:user_id).compact) + puts "After pinners: #{verified_ids.size} users" + + # Board accessors (accessed_at is touched when viewing boards) + verified_ids.merge(Access.where.not(accessed_at: nil).distinct.pluck(:user_id).compact) + puts "After board accessors: #{verified_ids.size} users" + + # Export requesters + verified_ids.merge(Account::Export.distinct.pluck(:user_id).compact) + puts "After export requesters: #{verified_ids.size} users" + + # Push subscribers + verified_ids.merge(Push::Subscription.distinct.pluck(:user_id).compact) + puts "After push subscribers: #{verified_ids.size} users" + + # Users who completed setup (name != email) + verified_ids.merge( + User.joins(:identity) + .where.not("users.name = identities.email_address") + .pluck(:id) + ) + puts "After setup completers: #{verified_ids.size} users" + + # Users whose identity has at least one session + verified_ids.merge( + User.where(identity_id: Session.distinct.select(:identity_id)).pluck(:id) + ) + puts "After identity sessions: #{verified_ids.size} users" + + verified_ids +end + +puts "Collecting verified user IDs..." +verified_user_ids = collect_verified_user_ids + +puts "\nFiltering to unverified users only..." +users_to_update = User.where(id: verified_user_ids.to_a) + .where(verified_at: nil) + .where(active: true) + .where.not(identity_id: nil) + .where.not(role: :system) + +update_count = users_to_update.count +puts "Found #{update_count} users to backfill" + +# Report remaining unverified users (before update) +remaining_before = User.where(verified_at: nil, active: true) + .where.not(identity_id: nil) + .where.not(role: :system) + .count +remaining_after = remaining_before - update_count +puts "\nCurrently unverified active users: #{remaining_before}" +puts "After backfill, remaining unverified: #{remaining_after}" +puts "These users will need to verify on next login." + +if update_count > 0 + puts "\nBackfilling verified_at..." + updated = users_to_update.update_all(verified_at: BACKFILL_TIMESTAMP) + puts "Updated #{updated} users" +end + +puts "\nDone!" From 1ad52d25906082f7c9ebac7bf3bf471a580bb620 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 5 Dec 2025 08:45:42 -0500 Subject: [PATCH 28/70] Only send notification emails to verified users Adds verified? check to bundling_emails? to prevent notification emails from being sent to users who have never authenticated. This closes the spam vector where bad actors could create users for known email addresses and trigger unwanted notifications by mentioning them. Co-Authored-By: Claude --- app/models/user/settings.rb | 2 +- test/models/user/settings_test.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/user/settings.rb b/app/models/user/settings.rb index 1651e9a64..598ede9c5 100644 --- a/app/models/user/settings.rb +++ b/app/models/user/settings.rb @@ -21,7 +21,7 @@ class User::Settings < ApplicationRecord end def bundling_emails? - !bundle_email_never? && !user.system? && user.active? + !bundle_email_never? && !user.system? && user.active? && user.verified? end def timezone diff --git a/test/models/user/settings_test.rb b/test/models/user/settings_test.rb index b72de92c5..69145c419 100644 --- a/test/models/user/settings_test.rb +++ b/test/models/user/settings_test.rb @@ -49,5 +49,9 @@ class User::SettingsTest < ActiveSupport::TestCase @user.update!(role: :member, active: false) assert_not @user.settings.bundling_emails?, "Inactive users should not receive bundled emails" + + @user.update!(active: true) + @user.update_column(:verified_at, nil) + assert_not @user.settings.bundling_emails?, "Unverified users should not receive bundled emails" end end From fc8ef333324a6b9d0a7a206050cd8287ef706178 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 5 Dec 2025 12:16:18 -0500 Subject: [PATCH 29/70] Show unverified status on user profile page Display "Unverified" with an explanation instead of the email address for users who haven't confirmed their identity. Hide the card links and activity timeline since unverified users won't have any activity. Co-Authored-By: Claude --- app/views/users/show.html.erb | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index eec6444a0..17f418e84 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -18,20 +18,25 @@

<%= @user.name %>

- <% if @user.active? %> - <%= mail_to @user.identity.email_address %> - <% else %> + <% if !@user.active? %> <%= @user.name %> is no longer on this account + <% elsif !@user.verified? %> + Unverified +
A sign-in code has been sent to this email address, but the user has not yet logged in to confirm their identity.
+ <% else %> + <%= mail_to @user.identity.email_address %> <% end %>
-
- <%= link_to "Which cards are assigned to #{me_or_you}?", - cards_path(assignee_ids: [ @user.id ], sorted_by: "newest"), class: "btn", data: { turbo_frame: "_top" } %> - <%= link_to "Which cards were added by #{me_or_you}?", - cards_path(creator_ids: [ @user.id ], sorted_by: "newest"), class: "btn", data: { turbo_frame: "_top" } %> -
+ <% if @user.verified? %> +
+ <%= link_to "Which cards are assigned to #{me_or_you}?", + cards_path(assignee_ids: [ @user.id ], sorted_by: "newest"), class: "btn", data: { turbo_frame: "_top" } %> + <%= link_to "Which cards were added by #{me_or_you}?", + cards_path(creator_ids: [ @user.id ], sorted_by: "newest"), class: "btn", data: { turbo_frame: "_top" } %> +
+ <% end %>
@@ -48,4 +53,6 @@ <% end %>
-<%= turbo_frame_tag "user_events", src: user_events_path(@user) %> +<% if @user.verified? %> + <%= turbo_frame_tag "user_events", src: user_events_path(@user) %> +<% end %> From 69f8216982cbb0f51376ddc77a0a0ca8e72d634f Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 5 Dec 2025 22:35:40 -0500 Subject: [PATCH 30/70] Pass correct params to webhook reactivation url helper. ref: https://app.fizzy.do/5986089/cards/3219 --- app/views/webhooks/show.html.erb | 2 +- test/controllers/webhooks_controller_test.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/views/webhooks/show.html.erb b/app/views/webhooks/show.html.erb index a98a8acf3..2fe01f070 100644 --- a/app/views/webhooks/show.html.erb +++ b/app/views/webhooks/show.html.erb @@ -22,7 +22,7 @@ <% unless @webhook.active? %>
- <%= button_to "Reactivate", board_webhook_activation_path(@webhook), method: :post %> + <%= button_to "Reactivate", board_webhook_activation_path(@webhook.board_id, @webhook), method: :post %>
<% end %> diff --git a/test/controllers/webhooks_controller_test.rb b/test/controllers/webhooks_controller_test.rb index 2cd427b5b..c9316172d 100644 --- a/test/controllers/webhooks_controller_test.rb +++ b/test/controllers/webhooks_controller_test.rb @@ -14,6 +14,10 @@ class WebhooksControllerTest < ActionDispatch::IntegrationTest webhook = webhooks(:active) get board_webhook_path(webhook.board, webhook) assert_response :success + + webhook = webhooks(:inactive) + get board_webhook_path(webhook.board, webhook) + assert_response :success end test "new" do @@ -63,6 +67,11 @@ class WebhooksControllerTest < ActionDispatch::IntegrationTest get edit_board_webhook_path(webhook.board, webhook) assert_response :success assert_select "form" + + webhook = webhooks(:inactive) + get edit_board_webhook_path(webhook.board, webhook) + assert_response :success + assert_select "form" end test "update with valid params" do From ab5964a3754504cde2033ee23b88179479f5e6ac Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 5 Dec 2025 20:45:48 -0800 Subject: [PATCH 31/70] Tune CSP: allow required external sources and user tools (#1977) External sources: - challenges.cloudflare.com for Turnstile (script-src, frame-src) - storage.basecamp.com for Active Storage (connect-src) User tools: loosen style/img/font/media/worker-src to avoid fighting accessibility extensions, privacy tools, and custom fonts. --- config/initializers/content_security_policy.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index fc63e9479..7a5b85df7 100644 --- a/config/initializers/content_security_policy.rb +++ b/config/initializers/content_security_policy.rb @@ -14,14 +14,19 @@ Rails.application.configure do config.content_security_policy do |policy| policy.default_src :self - policy.script_src :self + policy.script_src :self, "https://challenges.cloudflare.com" + policy.connect_src :self, "https://storage.basecamp.com" + policy.frame_src :self, "https://challenges.cloudflare.com" + + # Don't fight user tools: permit inline styles, data:/https: sources, and + # blob: workers for accessibility extensions, privacy tools, and custom fonts. policy.style_src :self, :unsafe_inline - policy.connect_src :self policy.img_src :self, "blob:", "data:", "https:" policy.font_src :self, "data:", "https:" policy.media_src :self, "blob:", "data:", "https:" - policy.object_src :none + policy.worker_src :self, "blob:" + policy.object_src :none policy.base_uri :none policy.form_action :self policy.frame_ancestors :self From af14feb8fca4d159b9a13dc349f13e70b8a2c571 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 5 Dec 2025 20:49:41 -0800 Subject: [PATCH 32/70] CSP gives env config precedence (#1976) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bit clearer, and simpler config wrangling: presence of ENV → use it. --- config/initializers/content_security_policy.rb | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index 7a5b85df7..fe37a5cde 100644 --- a/config/initializers/content_security_policy.rb +++ b/config/initializers/content_security_policy.rb @@ -4,9 +4,14 @@ # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy Rails.application.configure do - # Configure from environment variables (fizzy-saas can override by setting config.x values first) - config.x.content_security_policy.report_uri ||= ENV["CSP_REPORT_URI"] - config.x.content_security_policy.report_only ||= ENV["CSP_REPORT_ONLY"] == "true" + # Configure with environment variables with fallback to config.x values (via fizzy-sass) + report_uri = ENV.fetch("CSP_REPORT_URI") { config.x.content_security_policy.report_uri } + report_only = + if ENV.key?("CSP_REPORT_ONLY") + ENV["CSP_REPORT_ONLY"] == "true" + else + config.x.content_security_policy.report_only + end # Generate nonces for importmap and inline scripts config.content_security_policy_nonce_generator = ->(request) { SecureRandom.base64(16) } @@ -32,11 +37,9 @@ Rails.application.configure do policy.frame_ancestors :self # Specify URI for violation reports (e.g., Sentry CSP endpoint) - if report_uri = config.x.content_security_policy.report_uri - policy.report_uri report_uri - end + policy.report_uri report_uri if report_uri end # Report violations without enforcing the policy. - config.content_security_policy_report_only = config.x.content_security_policy.report_only + config.content_security_policy_report_only = report_only end unless ENV["DISABLE_CSP"] From e03c002efd45a0d7a8434d116b5883c2a7cee418 Mon Sep 17 00:00:00 2001 From: gijigae Date: Sat, 6 Dec 2025 17:26:35 +0900 Subject: [PATCH 33/70] Fix Japanese IME input handling for tags and form fields When typing in Japanese, the Enter key is used to confirm kanji conversion during IME composition. Previously, pressing Enter would immediately save the tag, which prevented users from completing the conversion. Changes: - Add isComposing check to navigable_list_controller.js Enter handler - Add IME composition tracking to form_controller.js - Add compositionstart/compositionend event handlers to tag, step, and reaction input fields - Add preventComposingSubmit action to prevent form submission during composition This fix supports all IME input methods (Japanese, Chinese, Korean, etc.) using standard browser composition events. --- app/javascript/controllers/form_controller.js | 17 +++++++++++++++++ .../controllers/navigable_list_controller.js | 3 +++ app/views/cards/comments/reactions/new.html.erb | 4 ++-- app/views/cards/display/perma/_steps.html.erb | 4 ++-- app/views/cards/taggings/new.html.erb | 4 ++-- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/app/javascript/controllers/form_controller.js b/app/javascript/controllers/form_controller.js index f917bd092..beca1f08f 100644 --- a/app/javascript/controllers/form_controller.js +++ b/app/javascript/controllers/form_controller.js @@ -8,10 +8,21 @@ export default class extends Controller { debounceTimeout: { type: Number, default: 300 } } + #isComposing = false + initialize() { this.debouncedSubmit = debounce(this.debouncedSubmit.bind(this), this.debounceTimeoutValue) } + // IME Composition tracking + compositionStart() { + this.#isComposing = true + } + + compositionEnd() { + this.#isComposing = false + } + submit() { this.element.requestSubmit() } @@ -32,6 +43,12 @@ export default class extends Controller { } } + preventComposingSubmit(event) { + if (this.#isComposing) { + event.preventDefault() + } + } + debouncedSubmit(event) { this.submit(event) } diff --git a/app/javascript/controllers/navigable_list_controller.js b/app/javascript/controllers/navigable_list_controller.js index 4af794c4b..bd5e87665 100644 --- a/app/javascript/controllers/navigable_list_controller.js +++ b/app/javascript/controllers/navigable_list_controller.js @@ -245,6 +245,9 @@ export default class extends Controller { } }, Enter(event) { + // Skip handling during IME composition (e.g., Japanese input) + if (event.isComposing) { return } + if (event.shiftKey) { this.#toggleCurrentItem(event) } else { diff --git a/app/views/cards/comments/reactions/new.html.erb b/app/views/cards/comments/reactions/new.html.erb index 38c6daf98..8cc5dda73 100644 --- a/app/views/cards/comments/reactions/new.html.erb +++ b/app/views/cards/comments/reactions/new.html.erb @@ -2,14 +2,14 @@ <%= form_with model: [ @comment.card, @comment, Reaction.new ], class: "reaction reaction__form expanded", html: { aria: { label: "New reaction" } }, - data: { controller: "form reaction-emoji", turbo_frame: dom_id(@comment, :reacting), action: "keydown.esc->form#cancel submit->form#preventEmptySubmit" } do |form| %> + data: { controller: "form reaction-emoji", turbo_frame: dom_id(@comment, :reacting), action: "keydown.esc->form#cancel submit->form#preventEmptySubmit submit->form#preventComposingSubmit" } do |form| %> <%= render "cards/comments/reactions/menu", comment: @comment %> diff --git a/app/views/cards/display/perma/_steps.html.erb b/app/views/cards/display/perma/_steps.html.erb index 36ce68fc5..9f65dec2a 100644 --- a/app/views/cards/display/perma/_steps.html.erb +++ b/app/views/cards/display/perma/_steps.html.erb @@ -4,8 +4,8 @@ <% unless card.closed? %>
  • - <%= form_with model: [card, Step.new], url: card_steps_path(card), class: "min-width", data: { controller: "form", action: "submit->form#preventEmptySubmit turbo:submit-end->form#reset" } do |form| %> - <%= form.text_field :content, class: "input step__content hide-focus-ring", placeholder: "Add a step…", autocomplete: "off", data: { form_target: "input", "1p-ignore": "true" }, aria: { label: "Add a step" } %> + <%= form_with model: [card, Step.new], url: card_steps_path(card), class: "min-width", data: { controller: "form", action: "submit->form#preventEmptySubmit submit->form#preventComposingSubmit turbo:submit-end->form#reset" } do |form| %> + <%= form.text_field :content, class: "input step__content hide-focus-ring", placeholder: "Add a step…", autocomplete: "off", data: { form_target: "input", "1p-ignore": "true", action: "compositionstart->form#compositionStart compositionend->form#compositionEnd" }, aria: { label: "Add a step" } %> <% end %>
    • <% end %> diff --git a/app/views/cards/taggings/new.html.erb b/app/views/cards/taggings/new.html.erb index 644b29d2b..b8ff47df6 100644 --- a/app/views/cards/taggings/new.html.erb +++ b/app/views/cards/taggings/new.html.erb @@ -11,10 +11,10 @@ <%= form_with url: card_taggings_path(@card), id: dom_id(@card, :tags_form), - data: { controller: "form", action: "submit->form#preventEmptySubmit" }, + data: { controller: "form", action: "submit->form#preventEmptySubmit submit->form#preventComposingSubmit" }, class: "flex flex-column gap-half full-width margin-block-half" do |form| %> <%= form.text_field :tag_title, placeholder: @tags.any? ? "Add a new tag or filter…" : "Name this tag…", class: "input txt-small full-width", - autocomplete: "off", autofocus: true, data: { filter_target: "input", form_target: "input", action: "input->filter#filter" } %> + autocomplete: "off", autofocus: true, data: { filter_target: "input", form_target: "input", action: "input->filter#filter compositionstart->form#compositionStart compositionend->form#compositionEnd" } %> <% end %>
      From 73c2b002115cba6be98972138b26147b6b693bd0 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Sat, 6 Dec 2025 11:04:55 +0100 Subject: [PATCH 34/70] Rever to using Ssrf instead of SSRF Ref: https://github.com/basecamp/fizzy/pull/1967#discussion_r2593750027 --- app/models/ssrf_protection.rb | 2 +- app/models/webhook/delivery.rb | 2 +- config/initializers/inflections.rb | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/models/ssrf_protection.rb b/app/models/ssrf_protection.rb index e43455b39..6e7c49c30 100644 --- a/app/models/ssrf_protection.rb +++ b/app/models/ssrf_protection.rb @@ -1,4 +1,4 @@ -module SSRFProtection +module SsrfProtection extend self DNS_RESOLUTION_TIMEOUT = 2 diff --git a/app/models/webhook/delivery.rb b/app/models/webhook/delivery.rb index 79db20b90..c5dab1497 100644 --- a/app/models/webhook/delivery.rb +++ b/app/models/webhook/delivery.rb @@ -70,7 +70,7 @@ class Webhook::Delivery < ApplicationRecord def resolved_ip return @resolved_ip if defined?(@resolved_ip) - @resolved_ip = SSRFProtection.resolve_public_ip(uri.host) + @resolved_ip = SsrfProtection.resolve_public_ip(uri.host) end def uri diff --git a/config/initializers/inflections.rb b/config/initializers/inflections.rb index 7e49acd7e..62ffe294f 100644 --- a/config/initializers/inflections.rb +++ b/config/initializers/inflections.rb @@ -13,7 +13,6 @@ # These inflection rules are supported but not enabled by default: ActiveSupport::Inflector.inflections(:en) do |inflect| inflect.acronym "SQLite" - inflect.acronym "SSRF" inflect.singular "quotas", "quota" inflect.plural "quota", "quotas" From 7d4775d24948554e5ee4abaf0565cc8a1b3b8fe1 Mon Sep 17 00:00:00 2001 From: AmiasYaska Date: Sat, 6 Dec 2025 14:32:05 +0300 Subject: [PATCH 35/70] Fix typo in back link: Actvity -> Activity --- app/views/events/day_timeline/columns/show.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/events/day_timeline/columns/show.html.erb b/app/views/events/day_timeline/columns/show.html.erb index 5df324465..dada2ebc4 100644 --- a/app/views/events/day_timeline/columns/show.html.erb +++ b/app/views/events/day_timeline/columns/show.html.erb @@ -2,7 +2,7 @@ <% content_for :header do %>
      - <%= back_link_to "Actvity", root_path, "keydown.left@document->hotkey#click keydown.esc@document->hotkey#click" %> + <%= back_link_to "Activity", root_path, "keydown.left@document->hotkey#click keydown.esc@document->hotkey#click" %>

      From e562c72ec9e0ade6504bda00be4f6a0000dbd16b Mon Sep 17 00:00:00 2001 From: Zoltan Hosszu Date: Sat, 6 Dec 2025 17:58:01 +0100 Subject: [PATCH 36/70] Small visual tweak for indented list style --- app/assets/stylesheets/rich-text-content.css | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/stylesheets/rich-text-content.css b/app/assets/stylesheets/rich-text-content.css index fdb8224fc..c9877f4bf 100644 --- a/app/assets/stylesheets/rich-text-content.css +++ b/app/assets/stylesheets/rich-text-content.css @@ -34,6 +34,11 @@ :where(li:has(li)) { list-style: none; + + :where(ol, ul) { + margin: 0; + padding-inline-start: 2ch; + } } :where(b, strong, .lexxy-content__bold) { From 529f5af5f686c98efe6c045d5b1a63349155cac9 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sat, 6 Dec 2025 10:08:22 -0800 Subject: [PATCH 37/70] CSP adherence: fix magic links console logging (#1989) * CSP: fix magic link console logging by including nonce in the script tag * Remove unused back-nav controller Unused since c0f842427df16114c4e2d9cc109c5820ab99cf73 Eliminates a potential CSP violation with javascript: URI --- .../controllers/back_navigation_controller.js | 13 ------------- app/views/sessions/magic_links/show.html.erb | 4 ++-- 2 files changed, 2 insertions(+), 15 deletions(-) delete mode 100644 app/javascript/controllers/back_navigation_controller.js diff --git a/app/javascript/controllers/back_navigation_controller.js b/app/javascript/controllers/back_navigation_controller.js deleted file mode 100644 index 8fad83419..000000000 --- a/app/javascript/controllers/back_navigation_controller.js +++ /dev/null @@ -1,13 +0,0 @@ -import { Controller } from "@hotwired/stimulus" - -export default class extends Controller { - static values = { fallbackDestination: String } - - connect() { - if (history.state.turbo?.restorationIndex > 0) { - this.element.href = "javascript:history.back()" - } else { - this.element.href = this.fallbackDestinationValue - } - } -} diff --git a/app/views/sessions/magic_links/show.html.erb b/app/views/sessions/magic_links/show.html.erb index 434bb93c3..7eca24264 100644 --- a/app/views/sessions/magic_links/show.html.erb +++ b/app/views/sessions/magic_links/show.html.erb @@ -17,9 +17,9 @@

    <% if Rails.env.development? %> - + <% end %> <% end %> <% content_for :footer do %> From 5865d79ed9ed6e6ed3e4ba6699e8731beca06830 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 6 Dec 2025 13:43:48 -0500 Subject: [PATCH 38/70] Display the verification code in the dev env This seems better and easier than the console log. --- app/assets/stylesheets/animation.css | 5 +++++ app/views/sessions/magic_links/show.html.erb | 10 ++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/animation.css b/app/assets/stylesheets/animation.css index 8c617a1d7..d6f05efa9 100644 --- a/app/assets/stylesheets/animation.css +++ b/app/assets/stylesheets/animation.css @@ -44,6 +44,11 @@ to { transform: translateY(0); } } + @keyframes slide-up-fade-in { + from { transform: translateY(2rem); opacity: 0; } + to { transform: translateY(0); opacity: 1; } + } + @keyframes slide-down { from { transform: translateY(0); } to { transform: translateY(2rem); } diff --git a/app/views/sessions/magic_links/show.html.erb b/app/views/sessions/magic_links/show.html.erb index 7eca24264..4e94f411e 100644 --- a/app/views/sessions/magic_links/show.html.erb +++ b/app/views/sessions/magic_links/show.html.erb @@ -16,10 +16,12 @@

    The code you receive will work for <%= distance_of_time_in_words(MagicLink::EXPIRATION_TIME) %>.

    -<% if Rails.env.development? %> - <%= javascript_tag do %> - console.log("Magic link code: <%= flash[:magic_link_code].presence || "not generated" %>"); - <% end %> +<% if Rails.env.development? && flash[:magic_link_code].present? %> +
    + Psst, here's your code: + <%= flash[:magic_link_code] %> + DEV +
    <% end %> <% content_for :footer do %> From 1883473de1b27be8926850f01c9cc8523faf9fdf Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 6 Dec 2025 14:33:44 -0500 Subject: [PATCH 39/70] Remove duplication in the list of supported avatar file types --- app/views/users/edit.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index a3b5845e8..45d4c437f 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -14,7 +14,7 @@ From c388f5ef209521a3fcc622225adb33a1c17373d0 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 6 Dec 2025 14:17:23 -0500 Subject: [PATCH 40/70] Display validation errors for user profiles. Specifically this will help people understand why their SVG avatar uploads are being rejected, and will keep the RecordInvalid exception out of Sentry logs. --- app/controllers/users_controller.rb | 7 +++++-- app/views/users/edit.html.erb | 11 +++++++++++ test/controllers/users_controller_test.rb | 22 ++++++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index ae800c75d..15f8ecd6f 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -9,8 +9,11 @@ class UsersController < ApplicationController end def update - @user.update! user_params - redirect_to @user + if @user.update(user_params) + redirect_to @user + else + render :edit, status: :unprocessable_entity + end end def destroy diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 45d4c437f..098ad729c 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -38,6 +38,17 @@ <%= link_to "Change email", new_user_email_address_path(user_id: Current.user.id), class: "btn btn--plain txt-link txt-small txt-nowrap" %> + + <% if @user.errors.any? %> +
    +
      + <% @user.errors.full_messages.each do |message| %> +
    • <%= message %>
      • + <% end %> +
    +
    + <% end %> + diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 081579dfc..385bacaf3 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -64,4 +64,26 @@ class UsersControllerTest < ActionDispatch::IntegrationTest delete user_path(users(:david)) assert_response :forbidden end + + test "update with invalid avatar shows validation error" do + sign_in_as :kevin + + svg_file = fixture_file_upload("avatar.svg", "image/svg+xml") + + put user_path(users(:kevin)), params: { user: { avatar: svg_file } } + assert_response :unprocessable_entity + assert_select "form[action='#{user_path(users(:kevin))}']" + assert_select ".txt-negative", text: /must be a JPEG, PNG, GIF, or WebP image/ + end + + test "update with valid avatar" do + sign_in_as :kevin + + png_file = fixture_file_upload("avatar.png", "image/png") + + put user_path(users(:kevin)), params: { user: { avatar: png_file } } + assert_redirected_to user_path(users(:kevin)) + assert users(:kevin).reload.avatar.attached? + assert_equal "image/png", users(:kevin).avatar.content_type + end end From 675600af72d95e5359a4ac3ca200a3b445ec65e8 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 6 Dec 2025 14:57:09 -0500 Subject: [PATCH 41/70] Improve presentation of validation errors --- app/views/signups/completions/new.html.erb | 5 +++-- app/views/users/edit.html.erb | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/views/signups/completions/new.html.erb b/app/views/signups/completions/new.html.erb index 971c39b1d..5be760843 100644 --- a/app/views/signups/completions/new.html.erb +++ b/app/views/signups/completions/new.html.erb @@ -9,8 +9,9 @@

    You're one step away. Just enter your name to get your own Fizzy account.

    <% if @signup.errors.any? %> -
    -
      +
      +

      Your changes couldn't be saved:

      +
        <% @signup.errors.full_messages.each do |message| %>
      • <%= message %>
        • <% end %> diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 098ad729c..c712dac58 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -40,8 +40,9 @@
      <% if @user.errors.any? %> -
      -
        +
        +

        Your changes couldn't be saved:

        +
          <% @user.errors.full_messages.each do |message| %>
        • <%= message %>
          • <% end %> From b439a7695b9e23eec53990c80f86916affafb880 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 6 Dec 2025 15:02:22 -0500 Subject: [PATCH 42/70] Render verification code in dev for signups, too Related to 5865d79e / #1991 --- app/controllers/signups_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/signups_controller.rb b/app/controllers/signups_controller.rb index 33da02f28..217221378 100644 --- a/app/controllers/signups_controller.rb +++ b/app/controllers/signups_controller.rb @@ -11,7 +11,8 @@ class SignupsController < ApplicationController end def create - Signup.new(signup_params).create_identity + magic_link = Signup.new(signup_params).create_identity + serve_development_magic_link(magic_link) redirect_to session_magic_link_path end From d747d1bfa35734ad68b0983035e78e0bf7b3a6e3 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 6 Dec 2025 16:05:03 -0500 Subject: [PATCH 43/70] Ensure we have good test coverage on Signup.create_identity because we're relying on it returning a MagicLink --- test/models/signup_test.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/models/signup_test.rb b/test/models/signup_test.rb index 30910bf81..d62b84769 100644 --- a/test/models/signup_test.rb +++ b/test/models/signup_test.rb @@ -4,12 +4,14 @@ class SignupTest < ActiveSupport::TestCase test "#create_identity" do signup = Signup.new(email_address: "brian@example.com") + magic_link = nil assert_difference -> { Identity.count }, 1 do assert_difference -> { MagicLink.count }, 1 do - assert signup.create_identity + magic_link = signup.create_identity end end + assert_kind_of MagicLink, magic_link assert_empty signup.errors assert signup.identity assert signup.identity.persisted? @@ -18,10 +20,12 @@ class SignupTest < ActiveSupport::TestCase assert_no_difference -> { Identity.count } do assert_difference -> { MagicLink.count }, 1 do - assert signup_existing.create_identity, "Should send magic link for existing identity" + magic_link = signup_existing.create_identity end end + assert_kind_of MagicLink, magic_link + signup_invalid = Signup.new(email_address: "") assert_raises do signup_invalid.create_identity From b2250b6d3588cac03956c877c360f388f003cf4c Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 6 Dec 2025 16:12:34 -0500 Subject: [PATCH 44/70] Ensure test coverage for all branches of JoinCodesController#create --- test/controllers/join_codes_controller_test.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/controllers/join_codes_controller_test.rb b/test/controllers/join_codes_controller_test.rb index 0a4bf5793..9558b605a 100644 --- a/test/controllers/join_codes_controller_test.rb +++ b/test/controllers/join_codes_controller_test.rb @@ -69,4 +69,17 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest assert_redirected_to new_users_verification_url(script_name: @account.slug) end + + test "create for different identity terminates existing session" do + sign_in_as :kevin + + assert_difference -> { Identity.count }, 1 do + assert_difference -> { User.count }, 1 do + post join_path(code: @join_code.code, script_name: @account.slug), params: { email_address: "new_user@example.com" } + end + end + + assert_redirected_to session_magic_link_url(script_name: nil) + assert_not_predicate cookies[:session_token], :present? + end end From 3ff45d77bbad3bc1d760536e72fd3bead4784c67 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 6 Dec 2025 16:46:33 -0500 Subject: [PATCH 45/70] Ensure test coverage for invalid Signup#complete --- test/models/signup_test.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/models/signup_test.rb b/test/models/signup_test.rb index d62b84769..cf7bd3359 100644 --- a/test/models/signup_test.rb +++ b/test/models/signup_test.rb @@ -52,4 +52,15 @@ class SignupTest < ActiveSupport::TestCase assert_not_empty signup_invalid.errors[:full_name] end end + + test "#complete with invalid data" do + Current.without_account do + signup = Signup.new + assert_not signup.complete + assert signup.errors[:full_name].any? + assert signup.errors[:identity].any? + assert_nil signup.account + assert_nil signup.user + end + end end From 95ba2c01b88b2c7a9284bb216256c5805cb8a6c3 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 6 Dec 2025 16:50:39 -0500 Subject: [PATCH 46/70] Add email validation to Signup --- app/models/signup.rb | 5 ++--- test/models/signup_test.rb | 9 +++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/models/signup.rb b/app/models/signup.rb index 15ac5873c..110c253ef 100644 --- a/app/models/signup.rb +++ b/app/models/signup.rb @@ -6,9 +6,8 @@ class Signup attr_accessor :full_name, :email_address, :identity attr_reader :account, :user - with_options on: :completion do - validates_presence_of :full_name, :identity - end + validates :email_address, format: { with: URI::MailTo::EMAIL_REGEXP }, on: :identity_creation + validates :full_name, :identity, presence: true, on: :completion def initialize(...) super diff --git a/test/models/signup_test.rb b/test/models/signup_test.rb index cf7bd3359..589bcd49a 100644 --- a/test/models/signup_test.rb +++ b/test/models/signup_test.rb @@ -1,6 +1,15 @@ require "test_helper" class SignupTest < ActiveSupport::TestCase + test "validates email format for identity creation" do + signup = Signup.new(email_address: "not-an-email") + assert_not signup.valid?(:identity_creation) + assert signup.errors[:email_address].any? + + signup = Signup.new(email_address: "valid@example.com") + assert signup.valid?(:identity_creation) + end + test "#create_identity" do signup = Signup.new(email_address: "brian@example.com") From 0160f215f2f35eb060ce408799e088094429cef8 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 6 Dec 2025 16:52:16 -0500 Subject: [PATCH 47/70] Validate email before creating identity during sign-up and sign-in Avoid Sentry exceptions when attackers try to stuff invalid emails. The browser performs form field validation that should normally prevent this from occurring, so we just return 422 without validation error messages. Also: - extract redirect_to_session_magic_link helper - some controller refactoring and cleanup --- app/controllers/concerns/authentication.rb | 6 +++++ app/controllers/join_codes_controller.rb | 16 ++++------- app/controllers/sessions_controller.rb | 17 ++++++------ app/controllers/signups_controller.rb | 9 ++++--- test/controllers/sessions_controller_test.rb | 28 +++++++++++++++----- test/controllers/signups_controller_test.rb | 16 ++++++----- test/test_helper.rb | 9 +++++++ 7 files changed, 64 insertions(+), 37 deletions(-) diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index 89eda5887..4cdbbdb69 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -97,6 +97,12 @@ module Authentication end end + def redirect_to_session_magic_link(magic_link, return_to: nil) + serve_development_magic_link(magic_link) + session[:return_to_after_authenticating] = return_to if return_to + redirect_to session_magic_link_url(script_name: nil) + end + def serve_development_magic_link(magic_link) if Rails.env.development? flash[:magic_link_code] = magic_link&.code diff --git a/app/controllers/join_codes_controller.rb b/app/controllers/join_codes_controller.rb index eb328df18..99298880c 100644 --- a/app/controllers/join_codes_controller.rb +++ b/app/controllers/join_codes_controller.rb @@ -20,8 +20,11 @@ class JoinCodesController < ApplicationController elsif identity == Current.identity redirect_to new_users_verification_url(script_name: @join_code.account.slug) else - logout_and_send_new_magic_link(identity) - redirect_to session_magic_link_url(script_name: nil) + terminate_session if Current.identity + + redirect_to_session_magic_link \ + identity.send_magic_link, + return_to: new_users_verification_url(script_name: @join_code.account.slug) end end @@ -37,13 +40,4 @@ class JoinCodesController < ApplicationController render :inactive, status: :gone end end - - def logout_and_send_new_magic_link(identity) - terminate_session if Current.identity - - magic_link = identity.send_magic_link - serve_development_magic_link(magic_link) - - session[:return_to_after_authenticating] = new_users_verification_url(script_name: @join_code.account.slug) - end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 5335f58f3..fb9fe0617 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -9,17 +9,16 @@ class SessionsController < ApplicationController end def create - identity = Identity.find_by_email_address(email_address) - - magic_link = if identity - identity.send_magic_link + if identity = Identity.find_by_email_address(email_address) + redirect_to_session_magic_link identity.send_magic_link else - Signup.new(email_address: email_address).create_identity + signup = Signup.new(email_address: email_address) + if signup.valid?(:identity_creation) + redirect_to_session_magic_link signup.create_identity + else + head :unprocessable_entity + end end - - serve_development_magic_link(magic_link) - - redirect_to session_magic_link_path end def destroy diff --git a/app/controllers/signups_controller.rb b/app/controllers/signups_controller.rb index 217221378..776421201 100644 --- a/app/controllers/signups_controller.rb +++ b/app/controllers/signups_controller.rb @@ -11,9 +11,12 @@ class SignupsController < ApplicationController end def create - magic_link = Signup.new(signup_params).create_identity - serve_development_magic_link(magic_link) - redirect_to session_magic_link_path + signup = Signup.new(signup_params) + if signup.valid?(:identity_creation) + redirect_to_session_magic_link signup.create_identity + else + head :unprocessable_entity + end end private diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index f2318bf46..7ebe4d518 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -36,15 +36,29 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest end end - private - test "destroy" do - sign_in_as :kevin - + test "create with invalid email address" do + # Avoid Sentry exceptions when attackers try to stuff invalid emails. The browser performs form + # field validation that should normally prevent this from occurring, so I'm not worried about + # returning proper validation errors. + without_action_dispatch_exception_handling do untenanted do - delete session_path + assert_no_difference -> { Identity.count } do + post session_path, params: { email_address: "not-a-valid-email" } + end - assert_redirected_to new_session_path - assert_not cookies[:session_token].present? + assert_response :unprocessable_entity end end + end + + test "destroy" do + sign_in_as :kevin + + untenanted do + delete session_path + + assert_redirected_to new_session_path + assert_not cookies[:session_token].present? + end + end end diff --git a/test/controllers/signups_controller_test.rb b/test/controllers/signups_controller_test.rb index 83fea8882..17314ee90 100644 --- a/test/controllers/signups_controller_test.rb +++ b/test/controllers/signups_controller_test.rb @@ -34,15 +34,17 @@ class SignupsControllerTest < ActionDispatch::IntegrationTest end end - test "create with email address containing blanks" do - untenanted do - assert_no_difference -> { Identity.count } do - assert_no_difference -> { MagicLink.count } do - post signup_path, params: { signup: { email_address: "sam smith@example.com" } } + test "create with invalid email address" do + without_action_dispatch_exception_handling do + untenanted do + assert_no_difference -> { Identity.count } do + assert_no_difference -> { MagicLink.count } do + post signup_path, params: { signup: { email_address: "not-a-valid-email" } } + end end - end - assert_response :unprocessable_entity + assert_response :unprocessable_entity + end end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 03cda99db..4cec836fd 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -61,6 +61,15 @@ class ActionDispatch::IntegrationTest setup do integration_session.default_url_options[:script_name] = "/#{ActiveRecord::FixtureSet.identify("37signals")}" end + + private + def without_action_dispatch_exception_handling + original = Rails.application.config.action_dispatch.show_exceptions + Rails.application.config.action_dispatch.show_exceptions = :none + yield + ensure + Rails.application.config.action_dispatch.show_exceptions = original + end end class ActionDispatch::SystemTestCase From 4c9d8edbc6dec41c3a879668cae39f8dd3c9b6c2 Mon Sep 17 00:00:00 2001 From: Felipe Felix Date: Sun, 7 Dec 2025 04:25:11 -0300 Subject: [PATCH 48/70] Test Notifications::SettingsController --- .../notifications/settings_controller_test.rb | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 test/controllers/notifications/settings_controller_test.rb diff --git a/test/controllers/notifications/settings_controller_test.rb b/test/controllers/notifications/settings_controller_test.rb new file mode 100644 index 000000000..d8472bb02 --- /dev/null +++ b/test/controllers/notifications/settings_controller_test.rb @@ -0,0 +1,24 @@ +require "test_helper" + +class Notifications::SettingsControllerTest < ActionDispatch::IntegrationTest + setup do + @user = users(:david) + + sign_in_as @user + end + + test "show" do + get notifications_settings_path + + assert_response :success + end + + test "update email frequency" do + assert_changes -> { @user.reload.settings.bundle_email_frequency }, from: "never", to: "every_few_hours" do + put notifications_settings_path, params: { user_settings: { bundle_email_frequency: "every_few_hours" } } + end + + assert_redirected_to notifications_settings_path + assert_equal "Settings updated", flash[:notice] + end +end From 31a41e66c27e54d248fc66761c167e139977aa85 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Sun, 7 Dec 2025 11:34:54 +0100 Subject: [PATCH 49/70] Make column have a proper ID instead of inferring it from the title --- app/models/user/day_timeline.rb | 10 +++++----- app/models/user/day_timeline/column.rb | 18 +++++++----------- app/views/events/day_timeline/_column.html.erb | 2 +- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/app/models/user/day_timeline.rb b/app/models/user/day_timeline.rb index 1fe4ed36e..4b30d24ca 100644 --- a/app/models/user/day_timeline.rb +++ b/app/models/user/day_timeline.rb @@ -30,15 +30,15 @@ class User::DayTimeline end def added_column - @added_column ||= build_column("Added", 1, events.where(action: %w[card_published card_reopened])) + @added_column ||= build_column(:added, "Added", 1, events.where(action: %w[card_published card_reopened])) end def updated_column - @updated_column ||= build_column("Updated", 2, events.where.not(action: %w[card_published card_closed card_reopened])) + @updated_column ||= build_column(:updated, "Updated", 2, events.where.not(action: %w[card_published card_closed card_reopened])) end def closed_column - @closed_column ||= build_column("Done", 3, events.where(action: "card_closed")) + @closed_column ||= build_column(:closed, "Done", 3, events.where(action: "card_closed")) end def cache_key @@ -84,8 +84,8 @@ class User::DayTimeline filtered_events.where(created_at: ...day.beginning_of_day).chronologically.last end - def build_column(base_title, index, events) - Column.new(self, base_title, index, events) + def build_column(id, base_title, index, events) + Column.new(self, id, base_title, index, events) end def window diff --git a/app/models/user/day_timeline/column.rb b/app/models/user/day_timeline/column.rb index ce87873fa..0f9261bbb 100644 --- a/app/models/user/day_timeline/column.rb +++ b/app/models/user/day_timeline/column.rb @@ -1,9 +1,10 @@ class User::DayTimeline::Column include ActionView::Helpers::TagHelper, ActionView::Helpers::OutputSafetyHelper, TimeHelper - attr_reader :index, :base_title, :day_timeline, :events + attr_reader :index, :id, :base_title, :day_timeline, :events - def initialize(day_timeline, base_title, index, events) + def initialize(day_timeline, id, base_title, index, events) + @id = id @day_timeline = day_timeline @base_title = base_title @index = index @@ -17,15 +18,6 @@ class User::DayTimeline::Column safe_join(parts, " ") end - def base_title_for_route - case base_title - when "Done" - "closed" - else - base_title.downcase - end - end - def events_by_hour limited_events.group_by { it.created_at.hour } end @@ -38,6 +30,10 @@ class User::DayTimeline::Column full_events_count - limited_events.count end + def to_param + id + end + private def limited_events @limited_events ||= events.limit(100).load diff --git a/app/views/events/day_timeline/_column.html.erb b/app/views/events/day_timeline/_column.html.erb index 509efd69d..50d35b4e2 100644 --- a/app/views/events/day_timeline/_column.html.erb +++ b/app/views/events/day_timeline/_column.html.erb @@ -3,7 +3,7 @@ <%= column.title %> <% if column.events_by_hour.any? %> - <%= link_to events_day_timeline_column_path(id: column.base_title_for_route, day: column.day_timeline.day.to_date), class: "events__maximize-button btn btn--circle txt-x-small borderless", data: { turbo_frame: "_top" } do %> + <%= link_to events_day_timeline_column_path(column, day: column.day_timeline.day.to_date), class: "events__maximize-button btn btn--circle txt-x-small borderless", data: { turbo_frame: "_top" } do %> <%= icon_tag "grid", class: "translucent" %> Expand column <% end %> From c8c91259c750db719d04e9f999aa2dbc32d3fa62 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Sun, 7 Dec 2025 12:06:03 +0100 Subject: [PATCH 50/70] Revert "Immediate avatar and embed variants" --- Gemfile.lock | 2 +- Gemfile.saas.lock | 6 ++--- app/models/concerns/attachments.rb | 9 ++++--- app/models/user/avatar.rb | 2 +- config/initializers/action_text.rb | 2 +- .../users/avatars_controller_test.rb | 8 ++----- .../notification/bundle_mailer_test.rb | 3 +-- test/models/comment_test.rb | 20 ---------------- test/models/user/avatar_test.rb | 24 ++----------------- 9 files changed, 17 insertions(+), 59 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 2f87d5cff..43d4e718b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -6,7 +6,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0 + revision: 3c3f6c8a253c8a0f346695374f927c9ab32fbefb branch: main specs: actioncable (8.2.0.alpha) diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index 18f80a22f..0c8880b08 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -75,7 +75,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0 + revision: 690ec8898318b8f50714e86676353ebe1551261e branch: main specs: actioncable (8.2.0.alpha) @@ -292,7 +292,7 @@ GEM actionview (>= 7.0.0) activesupport (>= 7.0.0) jmespath (1.6.2) - json (2.17.1) + json (2.16.0) jwt (3.1.2) base64 kamal (2.9.0) @@ -552,7 +552,7 @@ GEM ostruct stimulus-rails (1.3.4) railties (>= 6.0.0) - stringio (3.1.9) + stringio (3.1.8) thor (1.4.0) thruster (0.1.16) thruster (0.1.16-aarch64-linux) diff --git a/app/models/concerns/attachments.rb b/app/models/concerns/attachments.rb index bcc7303ae..1d1c2c4d6 100644 --- a/app/models/concerns/attachments.rb +++ b/app/models/concerns/attachments.rb @@ -1,10 +1,13 @@ module Attachments extend ActiveSupport::Concern - # Variants used by ActionText embeds. Processed immediately on attachment to avoid - # read replica issues (lazy variants would attempt writes on read replicas). + # The variants we use must all declared here so that we can preprocess them. # - # Patched into ActionText::RichText in config/initializers/action_text.rb + # If they are not preprocessed, then Rails will attempt to transform the image on-the-fly when + # they are first viewed, which may be on the read replica where writing to the database is not + # allowed. Chaos will ensue if that happens. + # + # These variants are patched into ActionText::RichText in config/initializers/action_text.rb VARIANTS = { # vipsthumbnail used to create sized image variants has a intent setting to manage colors during # resize. By setting an invalid intent value the gif-incompatible intent filtering is skipped and diff --git a/app/models/user/avatar.rb b/app/models/user/avatar.rb index c635edddb..a4b2ae631 100644 --- a/app/models/user/avatar.rb +++ b/app/models/user/avatar.rb @@ -5,7 +5,7 @@ module User::Avatar included do has_one_attached :avatar do |attachable| - attachable.variant :thumb, resize_to_fill: [ 256, 256 ], process: :immediately + attachable.variant :thumb, resize_to_fill: [ 256, 256 ] end validate :avatar_content_type_allowed diff --git a/config/initializers/action_text.rb b/config/initializers/action_text.rb index eee87de0d..b2724b1c7 100644 --- a/config/initializers/action_text.rb +++ b/config/initializers/action_text.rb @@ -7,7 +7,7 @@ module ActionText # This overrides the default :embeds association! has_many_attached :embeds do |attachable| ::Attachments::VARIANTS.each do |variant_name, variant_options| - attachable.variant variant_name, **variant_options, process: :immediately + attachable.variant variant_name, variant_options.merge(preprocessed: true) end end end diff --git a/test/controllers/users/avatars_controller_test.rb b/test/controllers/users/avatars_controller_test.rb index be4532448..3ded884b8 100644 --- a/test/controllers/users/avatars_controller_test.rb +++ b/test/controllers/users/avatars_controller_test.rb @@ -27,9 +27,7 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest end test "show own image redirects to the blob url" do - # Create blob separately to ensure file is uploaded before variant processing - blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") - users(:david).avatar.attach(blob) + users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") assert users(:david).avatar.attached? get user_avatar_path(users(:david)) @@ -38,9 +36,7 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest end test "show other image redirects to the blob url" do - # Create blob separately to ensure file is uploaded before variant processing - blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") - users(:kevin).avatar.attach(blob) + users(:kevin).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") assert users(:kevin).avatar.attached? get user_avatar_path(users(:kevin)) diff --git a/test/mailers/notification/bundle_mailer_test.rb b/test/mailers/notification/bundle_mailer_test.rb index 63647e68a..fbc4274c9 100644 --- a/test/mailers/notification/bundle_mailer_test.rb +++ b/test/mailers/notification/bundle_mailer_test.rb @@ -22,12 +22,11 @@ class Notification::BundleMailerTest < ActionMailer::TestCase end test "renders avatar with external image URL when avatar is attached" do - blob = ActiveStorage::Blob.create_and_upload!( + @user.avatar.attach( io: File.open(Rails.root.join("test", "fixtures", "files", "avatar.png")), filename: "avatar.png", content_type: "image/png" ) - @user.avatar.attach(blob) create_notification(@user) diff --git a/test/models/comment_test.rb b/test/models/comment_test.rb index 13a5e36c3..c519f8fbd 100644 --- a/test/models/comment_test.rb +++ b/test/models/comment_test.rb @@ -4,24 +4,4 @@ class CommentTest < ActiveSupport::TestCase setup do Current.session = sessions(:david) end - - test "rich text embed variants are processed immediately on attachment" do - comment = cards(:logo).comments.create!(body: "Check this out") - comment.body.body.attachables # force load - - blob = ActiveStorage::Blob.create_and_upload! \ - io: File.open(file_fixture("moon.jpg")), - filename: "moon.jpg", - content_type: "image/jpeg" - - comment.body.body = ActionText::Content.new(comment.body.body.to_html).append_attachables(blob) - comment.save! - - embed = comment.body.embeds.sole - - Attachments::VARIANTS.each_key do |variant_name| - variant = embed.variant(variant_name) - assert variant.processed?, "Expected #{variant_name} variant to be processed immediately" - end - end end diff --git a/test/models/user/avatar_test.rb b/test/models/user/avatar_test.rb index 58b78969c..96ea5bac7 100644 --- a/test/models/user/avatar_test.rb +++ b/test/models/user/avatar_test.rb @@ -2,8 +2,7 @@ require "test_helper" class User::AvatarTest < ActiveSupport::TestCase test "avatar_thumbnail returns variant for variable images" do - blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") - users(:david).avatar.attach(blob) + users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") assert users(:david).avatar.variable? assert_equal users(:david).avatar.variant(:thumb).blob, users(:david).avatar_thumbnail.blob @@ -17,8 +16,7 @@ class User::AvatarTest < ActiveSupport::TestCase end test "allows valid image content types" do - blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg", content_type: "image/jpeg") - users(:david).avatar.attach(blob) + users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg") assert users(:david).valid? end @@ -29,22 +27,4 @@ class User::AvatarTest < ActiveSupport::TestCase assert_not users(:david).valid? assert_includes users(:david).errors[:avatar], "must be a JPEG, PNG, GIF, or WebP image" end - - test "thumb variant is processed immediately on attachment" do - # Create blob separately to ensure file is uploaded before variant processing. - # - # Root cause: When ActiveStorage::Record uses `connects_to` for read replica support - # (as in SAAS mode), it creates a separate connection pool from application models. - # Since after_commit callbacks are tracked per connection pool, the callback order - # between User's pool (upload) and Attachment's pool (create_variants) isn't guaranteed. - # In MySQL/SAAS mode, the Attachment callback fires before the file is uploaded. - blob = ActiveStorage::Blob.create_and_upload!( - io: File.open(file_fixture("avatar.png")), - filename: "avatar.png", - content_type: "image/png" - ) - users(:david).avatar.attach(blob) - - assert users(:david).avatar.variant(:thumb).processed? - end end From 0b1a25c6b9173cb2f59b335f7f3ec18815c9687e Mon Sep 17 00:00:00 2001 From: user Date: Sun, 7 Dec 2025 20:47:25 +0900 Subject: [PATCH 51/70] Ignore RubyMine project files in .gitignore --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 38bd80d0d..7883e425c 100644 --- a/.gitignore +++ b/.gitignore @@ -41,3 +41,6 @@ /config/credentials/*.key .DS_Store + +# IDEA folder +.idea/ \ No newline at end of file From 661a7e5e2d3e7ec515ff697e5616be27c59872fd Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 7 Dec 2025 13:26:31 -0500 Subject: [PATCH 52/70] Validate email before creating identity during join code redemption Avoid Sentry exceptions when attackers try to stuff invalid emails. The browser performs form field validation that should normally prevent this from occurring, so we just return 422 without validation error messages. See similar change in #1996 for sign-in and sign-up --- app/controllers/join_codes_controller.rb | 25 +++++++++++++------ .../controllers/join_codes_controller_test.rb | 12 +++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/app/controllers/join_codes_controller.rb b/app/controllers/join_codes_controller.rb index 99298880c..8bd67c9dc 100644 --- a/app/controllers/join_codes_controller.rb +++ b/app/controllers/join_codes_controller.rb @@ -3,6 +3,7 @@ class JoinCodesController < ApplicationController before_action :set_join_code before_action :ensure_join_code_is_valid + before_action :set_identity, only: :create layout "public" @@ -10,25 +11,35 @@ class JoinCodesController < ApplicationController end def create - identity = Identity.find_or_create_by!(email_address: params.expect(:email_address)) + @join_code.redeem_if { |account| @identity.join(account) } + user = User.active.find_by!(account: @join_code.account, identity: @identity) - @join_code.redeem_if { |account| identity.join(account) } - user = User.active.find_by!(account: @join_code.account, identity: identity) - - if identity == Current.identity && user.setup? + if @identity == Current.identity && user.setup? redirect_to landing_url(script_name: @join_code.account.slug) - elsif identity == Current.identity + elsif @identity == Current.identity redirect_to new_users_verification_url(script_name: @join_code.account.slug) else terminate_session if Current.identity redirect_to_session_magic_link \ - identity.send_magic_link, + @identity.send_magic_link, return_to: new_users_verification_url(script_name: @join_code.account.slug) end end private + def set_identity + @identity = Identity.find_or_initialize_by(email_address: params.expect(:email_address)) + + if @identity.new_record? + if @identity.invalid? + head :unprocessable_entity + else + @identity.save! + end + end + end + def set_join_code @join_code ||= Account::JoinCode.find_by(code: params.expect(:code), account: Current.account) end diff --git a/test/controllers/join_codes_controller_test.rb b/test/controllers/join_codes_controller_test.rb index 9558b605a..415bde3e5 100644 --- a/test/controllers/join_codes_controller_test.rb +++ b/test/controllers/join_codes_controller_test.rb @@ -82,4 +82,16 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest assert_redirected_to session_magic_link_url(script_name: nil) assert_not_predicate cookies[:session_token], :present? end + + test "create with invalid email address" do + # Avoid Sentry exceptions when attackers try to stuff invalid emails into the system + without_action_dispatch_exception_handling do + assert_no_difference -> { Identity.count } do + assert_no_difference -> { User.count } do + post join_path(code: @join_code.code, script_name: @account.slug), params: { email_address: "not-a-valid-email" } + end + end + assert_response :unprocessable_entity + end + end end From 15983cede397712ff9690ab904518c89947aaf75 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Sun, 7 Dec 2025 20:02:59 +0100 Subject: [PATCH 53/70] No need to pass Currentl.user since it is the default value for the param --- app/controllers/cards/closures_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/cards/closures_controller.rb b/app/controllers/cards/closures_controller.rb index b89395cd5..b23600d91 100644 --- a/app/controllers/cards/closures_controller.rb +++ b/app/controllers/cards/closures_controller.rb @@ -2,12 +2,12 @@ class Cards::ClosuresController < ApplicationController include CardScoped def create - @card.close(user: Current.user) + @card.close render_card_replacement end def destroy - @card.reopen(user: Current.user) + @card.reopen render_card_replacement end end From 2110fba227dd3bd0b40b8981a4bb30c85dd3886c Mon Sep 17 00:00:00 2001 From: Robin Brandt Date: Mon, 8 Dec 2025 09:26:39 -0500 Subject: [PATCH 54/70] Bump mittens to 0.3.1 (#2006) --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 43d4e718b..e535c9864 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -267,7 +267,7 @@ GEM railties (>= 7.1) stimulus-rails turbo-rails - mittens (0.3.0) + mittens (0.3.1) mocha (2.8.2) ruby2_keywords (>= 0.0.5) msgpack (1.8.0) From c479235c6032b5373397541ec967ce51b0b0859d Mon Sep 17 00:00:00 2001 From: Alexander Zaytsev Date: Mon, 8 Dec 2025 15:54:36 +0100 Subject: [PATCH 55/70] Fix involvement button label --- app/helpers/accesses_helper.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/helpers/accesses_helper.rb b/app/helpers/accesses_helper.rb index 6ba7b4d3c..68caf6995 100644 --- a/app/helpers/accesses_helper.rb +++ b/app/helpers/accesses_helper.rb @@ -40,13 +40,15 @@ module AccessesHelper end def involvement_button(board, access, show_watchers, icon_only) + label_text = access.access_only? ? "Watch this" : "Stop watching" button_to( board_involvement_path(board), method: :put, params: { show_watchers: show_watchers, involvement: next_involvement(access.involvement), icon_only: icon_only }, - aria: { labelledby: dom_id(board, :involvement_label) }, title: access.access_only? ? "Watch this" : "Stop watching", + aria: { labelledby: dom_id(board, :involvement_label) }, + title: (label_text if icon_only), class: class_names("btn", { "btn--reversed": access.watching? && icon_only })) do icon_tag("notification-bell-#{icon_only ? 'reverse-' : nil}#{access.involvement.dasherize}") + - tag.span(class: class_names("txt-nowrap txt-uppercase", "for-screen-reader": icon_only), id: dom_id(board, :involvement_label)) + tag.span(label_text, class: class_names("txt-nowrap txt-uppercase", "for-screen-reader": icon_only), id: dom_id(board, :involvement_label)) end end From 642506e79ed5a0a4e977d1e3ea099c48b1ae0aaf Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 8 Dec 2025 11:52:17 -0500 Subject: [PATCH 56/70] Add datetime to the search results I find it helpful (especially when going through a long list of results) to know how far back in time I'm looking. A note on the timestamp being used: it's the search index timestamp. Using the card or comment `updated_at` is fraught because we do so much touching of records (e.g., comment is touched when a reaction is added; card is touched when a comment is added). Let's display the search record's `created_at`, which is the ordering used for the overall results. --- app/views/searches/_result.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/searches/_result.html.erb b/app/views/searches/_result.html.erb index b2b131910..ec110dd80 100644 --- a/app/views/searches/_result.html.erb +++ b/app/views/searches/_result.html.erb @@ -15,7 +15,7 @@ <% end %>
        - <%= result.card.board.name %> + <%= result.card.board.name %> · <%= local_datetime_tag(result.created_at, style: :timeordate) %>
        <% end %> From b88182c3ad3de478818c37890dab88c77b03268c Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 8 Dec 2025 10:01:28 -0800 Subject: [PATCH 57/70] Autotuner: use structured logging instead of Sentry (#2011) Move thousands of info-level Sentry events to logs. Query Loki rather than downsampling events: `event.action = "run.ruby-script", script.name = "autotuner"` --- config/initializers/autotuner.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/config/initializers/autotuner.rb b/config/initializers/autotuner.rb index ec8572b11..33eab9a05 100644 --- a/config/initializers/autotuner.rb +++ b/config/initializers/autotuner.rb @@ -3,12 +3,14 @@ Autotuner.enabled = true # This callback is called whenever a suggestion is provided by this gem. -# You can output this report to your logging pipeline, stdout, a file, -# or somewhere else! -Autotuner.reporter = proc do |report| +# Log to structured logging for query/analysis in Loki. This is called +# once per autotuner heuristic. +Autotuner.reporter = proc do |heuristic_report| + report = heuristic_report.to_s + Rails.logger.info "GCAUTOTUNE: #{report}" - if Fizzy.saas? - Sentry.capture_message "Autotuner suggestion", level: :info, extra: { report: report.to_s } - end + RailsStructuredLogging.instrument_script "autotuner" do + Rails.logger.info report.to_s + end if defined? RailsStructuredLogging end From 48730d67e035cc0e01c52054e25d66821ceb0906 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 8 Dec 2025 13:21:52 -0500 Subject: [PATCH 58/70] Disable board edit buttons for select all/none when not privileged ref: https://github.com/basecamp/fizzy/discussions/1992 --- ...ess_toggle.erb => _access_toggle.html.erb} | 0 app/views/boards/edit/_users.html.erb | 11 +++++----- test/controllers/boards_controller_test.rb | 21 +++++++++++++++++++ 3 files changed, 27 insertions(+), 5 deletions(-) rename app/views/boards/{_access_toggle.erb => _access_toggle.html.erb} (100%) diff --git a/app/views/boards/_access_toggle.erb b/app/views/boards/_access_toggle.html.erb similarity index 100% rename from app/views/boards/_access_toggle.erb rename to app/views/boards/_access_toggle.html.erb diff --git a/app/views/boards/edit/_users.html.erb b/app/views/boards/edit/_users.html.erb index 3214a2f2c..d8c16484a 100644 --- a/app/views/boards/edit/_users.html.erb +++ b/app/views/boards/edit/_users.html.erb @@ -1,3 +1,4 @@ +<% disabled = !Current.user.can_administer_board?(board) %>

        Who can access this board?

        @@ -16,7 +17,7 @@ @@ -26,14 +27,14 @@
        - + <%= button_tag "Select all", type: "button", class: "btn btn--plain txt-x-small txt-link font-weight-normal", data: { action: "click->toggle-class#checkAll" }, disabled: %> · - + <%= button_tag "Select none", type: "button", class: "btn btn--plain txt-x-small txt-link font-weight-normal", data: { action: "click->toggle-class#checkNone" }, disabled: %>
          - <%= access_toggles_for selected_users, selected: true, disabled: !Current.user.can_administer_board?(board) %> - <%= access_toggles_for unselected_users, selected: false, disabled: !Current.user.can_administer_board?(board) %> + <%= access_toggles_for selected_users, selected: true, disabled: %> + <%= access_toggles_for unselected_users, selected: false, disabled: %>
        <% end %> diff --git a/test/controllers/boards_controller_test.rb b/test/controllers/boards_controller_test.rb index 816a2e7b3..d9314775b 100644 --- a/test/controllers/boards_controller_test.rb +++ b/test/controllers/boards_controller_test.rb @@ -145,4 +145,25 @@ class BoardsControllerTest < ActionDispatch::IntegrationTest assert_response :forbidden end + + test "disables select all/none buttons for non-privileged user" do + logout_and_sign_in_as :jz + assert_not users(:jz).can_administer_board?(boards(:writebook)) + + get edit_board_path(boards(:writebook)) + + assert_response :success + assert_select "button[disabled]", text: "Select all" + assert_select "button[disabled]", text: "Select none" + end + + test "enables select all/none buttons for privileged user" do + assert users(:kevin).can_administer_board?(boards(:writebook)) + + get edit_board_path(boards(:writebook)) + + assert_response :success + assert_select "button:not([disabled])", text: "Select all" + assert_select "button:not([disabled])", text: "Select none" + end end From dcc005be346321a689b2a78ba4c51872b79bef9a Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 8 Dec 2025 11:00:40 -0800 Subject: [PATCH 59/70] Add lazy error context for Rails error reporter (#2014) Register a middleware with Rails.error that adds identity_id and account_id from Current attributes. Only evaluated when an error is actually reported, avoiding the cost on successful requests. --- Gemfile.saas.lock | 2 +- config/initializers/error_context.rb | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 config/initializers/error_context.rb diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index 0c8880b08..9100ff68a 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -21,7 +21,7 @@ GIT GIT remote: https://github.com/basecamp/fizzy-saas - revision: 3b30f3d56d8318bb9256469cb91c06a5a5c760a6 + revision: 43dbc896ce7b6a08194a92ddd1695d3f1ebf554b specs: fizzy-saas (0.1.0) audits1984 diff --git a/config/initializers/error_context.rb b/config/initializers/error_context.rb new file mode 100644 index 000000000..a1b207b6e --- /dev/null +++ b/config/initializers/error_context.rb @@ -0,0 +1,7 @@ +# Lazily add identity and account context to error reports. +# Only evaluated when an error is actually reported. +Rails.error.add_middleware ->(error, context:, **) do + context.merge \ + identity_id: Current.identity&.id, + account_id: Current.account&.external_account_id +end From 2352f30b614a5e2a09a8dc6aefb8a36742f6ff12 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Mon, 8 Dec 2025 20:05:03 +0100 Subject: [PATCH 60/70] Adopt a cooldown period for dependency updates As suggested by @flavorjones, and explained in: https://blog.yossarian.net/2025/11/21/We-should-all-be-using-dependency-cooldowns This applies: - 7 days as default: from the article, a 7-day cooldown would have prevented 8 out of 10 recent supply chain attacks. It seems reasonably long but not crazy long. - 14 days for major versions, to give some time to the community to find problems. --- .github/dependabot.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 86c66c646..f4f755614 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -20,8 +20,14 @@ updates: dependency-type: "development" schedule: interval: weekly + cooldown: + default-days: 7 + semver-major-days: 14 - package-ecosystem: github-actions directory: "/" schedule: interval: weekly open-pull-requests-limit: 10 + cooldown: + default-days: 7 + semver-major-days: 14 From abe48f0efa970d79ba46a6c1ed5dbe5a802007ec Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 8 Dec 2025 14:21:56 -0500 Subject: [PATCH 61/70] Ensure user toggles on board edit page are cached properly Not including the disabled flag as part of the cache resulted in issues like the one described in #1992 --- app/helpers/accesses_helper.rb | 2 +- test/controllers/boards_controller_test.rb | 24 ++++++++++++++++++++++ test/test_helper.rb | 2 +- test/test_helpers/caching_test_helper.rb | 14 +++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 test/test_helpers/caching_test_helper.rb diff --git a/app/helpers/accesses_helper.rb b/app/helpers/accesses_helper.rb index 68caf6995..69ef2efec 100644 --- a/app/helpers/accesses_helper.rb +++ b/app/helpers/accesses_helper.rb @@ -12,7 +12,7 @@ module AccessesHelper render partial: "boards/access_toggle", collection: users, as: :user, locals: { selected: selected, disabled: disabled }, - cached: ->(user) { [ user, selected ] } + cached: ->(user) { [ user, selected, disabled ] } end def access_involvement_advance_button(board, user, show_watchers: true, icon_only: false) diff --git a/test/controllers/boards_controller_test.rb b/test/controllers/boards_controller_test.rb index d9314775b..a7e3580de 100644 --- a/test/controllers/boards_controller_test.rb +++ b/test/controllers/boards_controller_test.rb @@ -166,4 +166,28 @@ class BoardsControllerTest < ActionDispatch::IntegrationTest assert_select "button:not([disabled])", text: "Select all" assert_select "button:not([disabled])", text: "Select none" end + + test "access toggle disabled state is cached correctly" do + board = boards(:writebook) + david = users(:david) + + with_actionview_partial_caching do + # privileged user + assert users(:kevin).can_administer_board?(board) + + get edit_board_path(board) + + assert_response :success + assert_select "input.switch__input[name='user_ids[]'][value='#{david.id}']:not([disabled])" + + # unprivileged user + logout_and_sign_in_as :jz + assert_not users(:jz).can_administer_board?(board) + + get edit_board_path(board) + + assert_response :success + assert_select "input.switch__input[name='user_ids[]'][value='#{david.id}'][disabled]" + end + end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 4cec836fd..bc856d815 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -44,7 +44,7 @@ module ActiveSupport fixtures :all include ActiveJob::TestHelper - include ActionTextTestHelper, CardTestHelper, ChangeTestHelper, SessionTestHelper + include ActionTextTestHelper, CachingTestHelper, CardTestHelper, ChangeTestHelper, SessionTestHelper include Turbo::Broadcastable::TestHelper setup do diff --git a/test/test_helpers/caching_test_helper.rb b/test/test_helpers/caching_test_helper.rb new file mode 100644 index 000000000..90dab64d5 --- /dev/null +++ b/test/test_helpers/caching_test_helper.rb @@ -0,0 +1,14 @@ +module CachingTestHelper + def with_actionview_partial_caching + was_cache = ActionView::PartialRenderer.collection_cache + was_perform_caching = ApplicationController.perform_caching + begin + ActionView::PartialRenderer.collection_cache = ActiveSupport::Cache.lookup_store(:memory_store) + ApplicationController.perform_caching = true + yield + ensure + ActionView::PartialRenderer.collection_cache = was_cache + ApplicationController.perform_caching = was_perform_caching + end + end +end From 3c4b838b259644414990e43fb61ec1acd15c1c32 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 8 Dec 2025 15:11:58 -0500 Subject: [PATCH 62/70] Fix Identity destruction callback ordering ref: #2003 Co-authored-by: Dylan --- app/models/identity.rb | 2 +- test/models/identity_test.rb | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/models/identity.rb b/app/models/identity.rb index ee4a323b2..bb69734b4 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -8,7 +8,7 @@ class Identity < ApplicationRecord has_one_attached :avatar - before_destroy :deactivate_users + before_destroy :deactivate_users, prepend: true validates :email_address, format: { with: URI::MailTo::EMAIL_REGEXP } normalizes :email_address, with: ->(value) { value.strip.downcase.presence } diff --git a/test/models/identity_test.rb b/test/models/identity_test.rb index 1c6c6b258..5b5763ffd 100644 --- a/test/models/identity_test.rb +++ b/test/models/identity_test.rb @@ -46,4 +46,19 @@ class IdentityTest < ActiveSupport::TestCase assert_equal identity.email_address, user.name end end + + test "destroy deactivates users before nullifying identity" do + identity = identities(:kevin) + user = users(:kevin) + + assert_predicate user, :active? + assert_predicate user.accesses, :any? + + identity.destroy! + user.reload + + assert_nil user.identity_id, "identity should be nullified" + assert_not_predicate user, :active? + assert_empty user.accesses, "user accesses should be removed" + end end From e1c9b7df1e5aaddb6b26c07f1b92d06ad768e312 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 8 Dec 2025 15:29:26 -0500 Subject: [PATCH 63/70] doc: update style doc reference for LLM agents --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 428c802a8..80bb887a2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -148,4 +148,4 @@ Use Chrome MCP tools to interact with the running dev app for UI testing and deb ## Coding style -Please read the separate file `STYLE.md` for some guidance on coding style. +@STYLE.md From 27585a75b211a8cc8fdba2c65749f0572ee8d380 Mon Sep 17 00:00:00 2001 From: Andy Smith Date: Mon, 8 Dec 2025 15:01:47 -0600 Subject: [PATCH 64/70] Wrap the overflow menu --- app/assets/stylesheets/lexxy.css | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/stylesheets/lexxy.css b/app/assets/stylesheets/lexxy.css index 7b647caca..04277d846 100644 --- a/app/assets/stylesheets/lexxy.css +++ b/app/assets/stylesheets/lexxy.css @@ -149,6 +149,7 @@ border-radius: 0.5ch; box-shadow: var(--shadow); display: flex; + flex-wrap: wrap; inset-inline-end: 0; padding: 4px; position: absolute; From bc8ecc3e91df43f74e0ba7a5422fcece9a984021 Mon Sep 17 00:00:00 2001 From: Andy Smith Date: Mon, 8 Dec 2025 15:59:58 -0600 Subject: [PATCH 65/70] Account for browser button styled and adjust mobile padding for perma BG --- app/assets/stylesheets/card-perma.css | 6 +++++- app/assets/stylesheets/lexxy.css | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/card-perma.css b/app/assets/stylesheets/card-perma.css index 150dde9a1..3132d886f 100644 --- a/app/assets/stylesheets/card-perma.css +++ b/app/assets/stylesheets/card-perma.css @@ -127,7 +127,7 @@ } @media (max-width: 799px) { - --padding-inline: var(--main-padding); + --padding-inline: 1.5ch; column-gap: 0; grid-template-areas: @@ -150,6 +150,10 @@ border-radius: 0.2em; grid-area: card; padding: clamp(2rem, 4vw, var(--padding-block)); + + @media (max-width: 799px) { + padding-inline: var(--padding-inline); + } } .card-perma__actions { diff --git a/app/assets/stylesheets/lexxy.css b/app/assets/stylesheets/lexxy.css index 04277d846..cc79ca76a 100644 --- a/app/assets/stylesheets/lexxy.css +++ b/app/assets/stylesheets/lexxy.css @@ -234,10 +234,11 @@ background: var(--color-canvas); border: none; border-radius: 0.5ch; + color: inherit; display: grid; font-weight: 500; inline-size: var(--lexxy-dropdown-btn-size); - place-items: center; + place-content: center; position: relative; outline: none; From 3cdcd3f00c2dd8c7c2c03918a77493cad043727e Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Mon, 8 Dec 2025 16:03:39 -0600 Subject: [PATCH 66/70] Should be in global gitignore --- .gitignore | 3 --- 1 file changed, 3 deletions(-) diff --git a/.gitignore b/.gitignore index 7883e425c..38bd80d0d 100644 --- a/.gitignore +++ b/.gitignore @@ -41,6 +41,3 @@ /config/credentials/*.key .DS_Store - -# IDEA folder -.idea/ \ No newline at end of file From cb0e9b99622cfed0c3cc57d735275493a695d0ff Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Mon, 8 Dec 2025 23:17:06 +0100 Subject: [PATCH 67/70] Immediate avatar and embed variants (#2002) Reverts #2001 Restores #1955 --- Gemfile.lock | 2 +- Gemfile.saas.lock | 6 ++--- app/models/concerns/attachments.rb | 9 +++---- app/models/user/avatar.rb | 2 +- config/initializers/action_text.rb | 2 +- .../users/avatars_controller_test.rb | 8 +++++-- test/controllers/users_controller_test.rb | 5 ++-- .../notification/bundle_mailer_test.rb | 3 ++- test/models/comment_test.rb | 20 ++++++++++++++++ test/models/user/avatar_test.rb | 24 +++++++++++++++++-- 10 files changed, 62 insertions(+), 19 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index e535c9864..e4f364c1c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -6,7 +6,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: 3c3f6c8a253c8a0f346695374f927c9ab32fbefb + revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0 branch: main specs: actioncable (8.2.0.alpha) diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index 9100ff68a..f093ac677 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -75,7 +75,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: 690ec8898318b8f50714e86676353ebe1551261e + revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0 branch: main specs: actioncable (8.2.0.alpha) @@ -292,7 +292,7 @@ GEM actionview (>= 7.0.0) activesupport (>= 7.0.0) jmespath (1.6.2) - json (2.16.0) + json (2.17.1) jwt (3.1.2) base64 kamal (2.9.0) @@ -552,7 +552,7 @@ GEM ostruct stimulus-rails (1.3.4) railties (>= 6.0.0) - stringio (3.1.8) + stringio (3.1.9) thor (1.4.0) thruster (0.1.16) thruster (0.1.16-aarch64-linux) diff --git a/app/models/concerns/attachments.rb b/app/models/concerns/attachments.rb index 1d1c2c4d6..bcc7303ae 100644 --- a/app/models/concerns/attachments.rb +++ b/app/models/concerns/attachments.rb @@ -1,13 +1,10 @@ module Attachments extend ActiveSupport::Concern - # The variants we use must all declared here so that we can preprocess them. + # Variants used by ActionText embeds. Processed immediately on attachment to avoid + # read replica issues (lazy variants would attempt writes on read replicas). # - # If they are not preprocessed, then Rails will attempt to transform the image on-the-fly when - # they are first viewed, which may be on the read replica where writing to the database is not - # allowed. Chaos will ensue if that happens. - # - # These variants are patched into ActionText::RichText in config/initializers/action_text.rb + # Patched into ActionText::RichText in config/initializers/action_text.rb VARIANTS = { # vipsthumbnail used to create sized image variants has a intent setting to manage colors during # resize. By setting an invalid intent value the gif-incompatible intent filtering is skipped and diff --git a/app/models/user/avatar.rb b/app/models/user/avatar.rb index a4b2ae631..c635edddb 100644 --- a/app/models/user/avatar.rb +++ b/app/models/user/avatar.rb @@ -5,7 +5,7 @@ module User::Avatar included do has_one_attached :avatar do |attachable| - attachable.variant :thumb, resize_to_fill: [ 256, 256 ] + attachable.variant :thumb, resize_to_fill: [ 256, 256 ], process: :immediately end validate :avatar_content_type_allowed diff --git a/config/initializers/action_text.rb b/config/initializers/action_text.rb index b2724b1c7..eee87de0d 100644 --- a/config/initializers/action_text.rb +++ b/config/initializers/action_text.rb @@ -7,7 +7,7 @@ module ActionText # This overrides the default :embeds association! has_many_attached :embeds do |attachable| ::Attachments::VARIANTS.each do |variant_name, variant_options| - attachable.variant variant_name, variant_options.merge(preprocessed: true) + attachable.variant variant_name, **variant_options, process: :immediately end end end diff --git a/test/controllers/users/avatars_controller_test.rb b/test/controllers/users/avatars_controller_test.rb index 3ded884b8..be4532448 100644 --- a/test/controllers/users/avatars_controller_test.rb +++ b/test/controllers/users/avatars_controller_test.rb @@ -27,7 +27,9 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest end test "show own image redirects to the blob url" do - users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + # Create blob separately to ensure file is uploaded before variant processing + blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + users(:david).avatar.attach(blob) assert users(:david).avatar.attached? get user_avatar_path(users(:david)) @@ -36,7 +38,9 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest end test "show other image redirects to the blob url" do - users(:kevin).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + # Create blob separately to ensure file is uploaded before variant processing + blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + users(:kevin).avatar.attach(blob) assert users(:kevin).avatar.attached? get user_avatar_path(users(:kevin)) diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 385bacaf3..d8441d033 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -79,9 +79,10 @@ class UsersControllerTest < ActionDispatch::IntegrationTest test "update with valid avatar" do sign_in_as :kevin - png_file = fixture_file_upload("avatar.png", "image/png") + # Create blob separately to ensure file is uploaded before variant processing + blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("avatar.png")), filename: "avatar.png", content_type: "image/png") - put user_path(users(:kevin)), params: { user: { avatar: png_file } } + put user_path(users(:kevin)), params: { user: { avatar: blob.signed_id } } assert_redirected_to user_path(users(:kevin)) assert users(:kevin).reload.avatar.attached? assert_equal "image/png", users(:kevin).avatar.content_type diff --git a/test/mailers/notification/bundle_mailer_test.rb b/test/mailers/notification/bundle_mailer_test.rb index fbc4274c9..63647e68a 100644 --- a/test/mailers/notification/bundle_mailer_test.rb +++ b/test/mailers/notification/bundle_mailer_test.rb @@ -22,11 +22,12 @@ class Notification::BundleMailerTest < ActionMailer::TestCase end test "renders avatar with external image URL when avatar is attached" do - @user.avatar.attach( + blob = ActiveStorage::Blob.create_and_upload!( io: File.open(Rails.root.join("test", "fixtures", "files", "avatar.png")), filename: "avatar.png", content_type: "image/png" ) + @user.avatar.attach(blob) create_notification(@user) diff --git a/test/models/comment_test.rb b/test/models/comment_test.rb index c519f8fbd..13a5e36c3 100644 --- a/test/models/comment_test.rb +++ b/test/models/comment_test.rb @@ -4,4 +4,24 @@ class CommentTest < ActiveSupport::TestCase setup do Current.session = sessions(:david) end + + test "rich text embed variants are processed immediately on attachment" do + comment = cards(:logo).comments.create!(body: "Check this out") + comment.body.body.attachables # force load + + blob = ActiveStorage::Blob.create_and_upload! \ + io: File.open(file_fixture("moon.jpg")), + filename: "moon.jpg", + content_type: "image/jpeg" + + comment.body.body = ActionText::Content.new(comment.body.body.to_html).append_attachables(blob) + comment.save! + + embed = comment.body.embeds.sole + + Attachments::VARIANTS.each_key do |variant_name| + variant = embed.variant(variant_name) + assert variant.processed?, "Expected #{variant_name} variant to be processed immediately" + end + end end diff --git a/test/models/user/avatar_test.rb b/test/models/user/avatar_test.rb index 96ea5bac7..58b78969c 100644 --- a/test/models/user/avatar_test.rb +++ b/test/models/user/avatar_test.rb @@ -2,7 +2,8 @@ require "test_helper" class User::AvatarTest < ActiveSupport::TestCase test "avatar_thumbnail returns variant for variable images" do - users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + users(:david).avatar.attach(blob) assert users(:david).avatar.variable? assert_equal users(:david).avatar.variant(:thumb).blob, users(:david).avatar_thumbnail.blob @@ -16,7 +17,8 @@ class User::AvatarTest < ActiveSupport::TestCase end test "allows valid image content types" do - users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg") + blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg", content_type: "image/jpeg") + users(:david).avatar.attach(blob) assert users(:david).valid? end @@ -27,4 +29,22 @@ class User::AvatarTest < ActiveSupport::TestCase assert_not users(:david).valid? assert_includes users(:david).errors[:avatar], "must be a JPEG, PNG, GIF, or WebP image" end + + test "thumb variant is processed immediately on attachment" do + # Create blob separately to ensure file is uploaded before variant processing. + # + # Root cause: When ActiveStorage::Record uses `connects_to` for read replica support + # (as in SAAS mode), it creates a separate connection pool from application models. + # Since after_commit callbacks are tracked per connection pool, the callback order + # between User's pool (upload) and Attachment's pool (create_variants) isn't guaranteed. + # In MySQL/SAAS mode, the Attachment callback fires before the file is uploaded. + blob = ActiveStorage::Blob.create_and_upload!( + io: File.open(file_fixture("avatar.png")), + filename: "avatar.png", + content_type: "image/png" + ) + users(:david).avatar.attach(blob) + + assert users(:david).avatar.variant(:thumb).processed? + end end From 49c4f2adc6069d8e58f3091a797e9182d85ebbb6 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 8 Dec 2025 14:44:49 -0800 Subject: [PATCH 68/70] Fix ActiveStorage FileNotFoundError with immediate variant processing (#2022) When ActiveStorage::Record uses `connects_to` for read replica support, it creates a separate connection pool from ApplicationRecord. This causes `after_commit` callbacks to fire in non-deterministic order - the Attachment's `create_variants` callback can fire before the User model's upload callback completes, resulting in FileNotFoundError. The fix removes replica connection configuration from ActiveStorage::Record so it shares the same connection pool as application models, ensuring proper callback ordering. Also reverts test workarounds that were added to work around this issue, since the root cause is now fixed. See: https://github.com/rails/rails/issues/53694 --- config/initializers/active_storage.rb | 14 ++++++++++--- .../users/avatars_controller_test.rb | 8 ++------ test/controllers/users_controller_test.rb | 5 ++--- .../notification/bundle_mailer_test.rb | 3 +-- test/models/user/avatar_test.rb | 20 +++---------------- 5 files changed, 19 insertions(+), 31 deletions(-) diff --git a/config/initializers/active_storage.rb b/config/initializers/active_storage.rb index d00bd74bf..4a4a0343d 100644 --- a/config/initializers/active_storage.rb +++ b/config/initializers/active_storage.rb @@ -4,9 +4,17 @@ ActiveSupport.on_load(:active_storage_blob) do end end -ActiveSupport.on_load(:active_storage_record) do - configure_replica_connections -end +# Don't configure replica connections for ActiveStorage::Record. +# When ActiveStorage uses `connects_to`, it creates a separate connection pool +# from ApplicationRecord. This causes after_commit callbacks to fire in +# non-deterministic order - the Attachment's create_variants callback can fire +# before the User model's upload callback, causing FileNotFoundError when +# using `process: :immediately` for variants. +# See: https://github.com/rails/rails/issues/53694 +# +# ActiveSupport.on_load(:active_storage_record) do +# configure_replica_connections +# end module ActiveStorageControllerExtensions extend ActiveSupport::Concern diff --git a/test/controllers/users/avatars_controller_test.rb b/test/controllers/users/avatars_controller_test.rb index be4532448..3ded884b8 100644 --- a/test/controllers/users/avatars_controller_test.rb +++ b/test/controllers/users/avatars_controller_test.rb @@ -27,9 +27,7 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest end test "show own image redirects to the blob url" do - # Create blob separately to ensure file is uploaded before variant processing - blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") - users(:david).avatar.attach(blob) + users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") assert users(:david).avatar.attached? get user_avatar_path(users(:david)) @@ -38,9 +36,7 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest end test "show other image redirects to the blob url" do - # Create blob separately to ensure file is uploaded before variant processing - blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") - users(:kevin).avatar.attach(blob) + users(:kevin).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") assert users(:kevin).avatar.attached? get user_avatar_path(users(:kevin)) diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index d8441d033..385bacaf3 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -79,10 +79,9 @@ class UsersControllerTest < ActionDispatch::IntegrationTest test "update with valid avatar" do sign_in_as :kevin - # Create blob separately to ensure file is uploaded before variant processing - blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("avatar.png")), filename: "avatar.png", content_type: "image/png") + png_file = fixture_file_upload("avatar.png", "image/png") - put user_path(users(:kevin)), params: { user: { avatar: blob.signed_id } } + put user_path(users(:kevin)), params: { user: { avatar: png_file } } assert_redirected_to user_path(users(:kevin)) assert users(:kevin).reload.avatar.attached? assert_equal "image/png", users(:kevin).avatar.content_type diff --git a/test/mailers/notification/bundle_mailer_test.rb b/test/mailers/notification/bundle_mailer_test.rb index 63647e68a..fbc4274c9 100644 --- a/test/mailers/notification/bundle_mailer_test.rb +++ b/test/mailers/notification/bundle_mailer_test.rb @@ -22,12 +22,11 @@ class Notification::BundleMailerTest < ActionMailer::TestCase end test "renders avatar with external image URL when avatar is attached" do - blob = ActiveStorage::Blob.create_and_upload!( + @user.avatar.attach( io: File.open(Rails.root.join("test", "fixtures", "files", "avatar.png")), filename: "avatar.png", content_type: "image/png" ) - @user.avatar.attach(blob) create_notification(@user) diff --git a/test/models/user/avatar_test.rb b/test/models/user/avatar_test.rb index 58b78969c..ed502d7ab 100644 --- a/test/models/user/avatar_test.rb +++ b/test/models/user/avatar_test.rb @@ -2,8 +2,7 @@ require "test_helper" class User::AvatarTest < ActiveSupport::TestCase test "avatar_thumbnail returns variant for variable images" do - blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") - users(:david).avatar.attach(blob) + users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") assert users(:david).avatar.variable? assert_equal users(:david).avatar.variant(:thumb).blob, users(:david).avatar_thumbnail.blob @@ -17,8 +16,7 @@ class User::AvatarTest < ActiveSupport::TestCase end test "allows valid image content types" do - blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg", content_type: "image/jpeg") - users(:david).avatar.attach(blob) + users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg", content_type: "image/jpeg") assert users(:david).valid? end @@ -31,19 +29,7 @@ class User::AvatarTest < ActiveSupport::TestCase end test "thumb variant is processed immediately on attachment" do - # Create blob separately to ensure file is uploaded before variant processing. - # - # Root cause: When ActiveStorage::Record uses `connects_to` for read replica support - # (as in SAAS mode), it creates a separate connection pool from application models. - # Since after_commit callbacks are tracked per connection pool, the callback order - # between User's pool (upload) and Attachment's pool (create_variants) isn't guaranteed. - # In MySQL/SAAS mode, the Attachment callback fires before the file is uploaded. - blob = ActiveStorage::Blob.create_and_upload!( - io: File.open(file_fixture("avatar.png")), - filename: "avatar.png", - content_type: "image/png" - ) - users(:david).avatar.attach(blob) + users(:david).avatar.attach(io: File.open(file_fixture("avatar.png")), filename: "avatar.png", content_type: "image/png") assert users(:david).avatar.variant(:thumb).processed? end From b23a5d0b7c68ef8a816d609b04ba01f31b3d5e48 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 8 Dec 2025 23:27:36 -0800 Subject: [PATCH 69/70] Fix flaky tests caused by leaky show_exceptions twiddling (#2028) Rails memoizes `@app_env_config`, so running push subscriptions test after join codes test will result in running with `show_exceptions: :none`, causing RecordInvalid to raise instead of returning 422 status. References #1996 --- test/test_helper.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test_helper.rb b/test/test_helper.rb index bc856d815..d18f4c9c4 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -66,9 +66,11 @@ class ActionDispatch::IntegrationTest def without_action_dispatch_exception_handling original = Rails.application.config.action_dispatch.show_exceptions Rails.application.config.action_dispatch.show_exceptions = :none + Rails.application.instance_variable_set(:@app_env_config, nil) # Clear memoized env_config yield ensure Rails.application.config.action_dispatch.show_exceptions = original + Rails.application.instance_variable_set(:@app_env_config, nil) # Reset env_config end end From 08896deb328628cf4c0f244d07848cd68f671068 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 8 Dec 2025 23:33:46 -0800 Subject: [PATCH 70/70] bin/ci: run OSS suite in SAAS mode for full coverage (#2029) --- config/ci.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/ci.rb b/config/ci.rb index 053fe2682..a99248a93 100644 --- a/config/ci.rb +++ b/config/ci.rb @@ -19,6 +19,8 @@ CI.run do if Fizzy.saas? step "Tests: SaaS", "#{SAAS_ENV} bin/rails test" step "Tests: SaaS System", "#{SAAS_ENV} #{SYSTEM_TEST_ENV} bin/rails test:system" + step "Tests: OSS", "#{OSS_ENV} bin/rails test" + step "Tests: OSS System", "#{OSS_ENV} #{SYSTEM_TEST_ENV} bin/rails test:system" else step "Tests: SQLite", "#{OSS_ENV} bin/rails test" step "Tests: SQLite System", "#{OSS_ENV} #{SYSTEM_TEST_ENV} bin/rails test:system"