diff --git a/Gemfile b/Gemfile index 7f7a463bb..e8a5f8514 100644 --- a/Gemfile +++ b/Gemfile @@ -1,8 +1,9 @@ source "https://rubygems.org" -gem "rails", github: "rails/rails", branch: "main" git_source(:bc) { |repo| "https://github.com/basecamp/#{repo}" } +gem "rails", github: "rails/rails", branch: "ast-immediate-variants-process-locally" + # Assets & front end gem "importmap-rails" gem "propshaft" @@ -27,7 +28,7 @@ gem "rqrcode" gem "redcarpet" gem "rouge" gem "jbuilder" -gem "lexxy" +gem "lexxy", bc: "lexxy" gem "image_processing", "~> 1.14" gem "platform_agent" gem "aws-sdk-s3", require: false diff --git a/Gemfile.lock b/Gemfile.lock index e4f364c1c..ea4794b5a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,10 @@ +GIT + remote: https://github.com/basecamp/lexxy + revision: d292280b6d2c5d8a924327adf8bb9f0b25953539 + specs: + lexxy (0.1.23.beta) + rails (>= 8.0.2) + GIT remote: https://github.com/basecamp/useragent revision: 433ca320a42db1266c4b89df74d0abdb9a880c5e @@ -6,8 +13,8 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0 - branch: main + revision: bf81d40a91880c059ce9d0447219385a2c8e4a15 + branch: ast-immediate-variants-process-locally specs: actioncable (8.2.0.alpha) actionpack (= 8.2.0.alpha) @@ -237,8 +244,6 @@ GEM logger (~> 1.6) letter_opener (1.10.0) launchy (>= 2.2, < 4) - lexxy (0.1.23.beta) - rails (>= 8.0.2) lint_roller (1.1.0) logger (1.7.0) loofah (2.24.1) @@ -518,7 +523,7 @@ DEPENDENCIES jbuilder kamal letter_opener - lexxy + lexxy! mission_control-jobs mittens mocha diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index f093ac677..aabe04258 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -41,6 +41,13 @@ GIT yabeda-puma-plugin yabeda-rails (>= 0.10) +GIT + remote: https://github.com/basecamp/lexxy + revision: d292280b6d2c5d8a924327adf8bb9f0b25953539 + specs: + lexxy (0.1.23.beta) + rails (>= 8.0.2) + GIT remote: https://github.com/basecamp/queenbee-plugin revision: 15faf03a876c5e66b67753d2e1ddb24f1eb5abb2 @@ -75,8 +82,8 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0 - branch: main + revision: bf81d40a91880c059ce9d0447219385a2c8e4a15 + branch: ast-immediate-variants-process-locally specs: actioncable (8.2.0.alpha) actionpack (= 8.2.0.alpha) @@ -313,8 +320,6 @@ GEM logger (~> 1.6) letter_opener (1.10.0) launchy (>= 2.2, < 4) - lexxy (0.1.23.beta) - rails (>= 8.0.2) lint_roller (1.1.0) logger (1.7.0) loofah (2.24.1) @@ -653,7 +658,7 @@ DEPENDENCIES jbuilder kamal letter_opener - lexxy + lexxy! mission_control-jobs mittens mocha diff --git a/app/controllers/users/push_subscriptions_controller.rb b/app/controllers/users/push_subscriptions_controller.rb index bf5e5f87e..b511a6b19 100644 --- a/app/controllers/users/push_subscriptions_controller.rb +++ b/app/controllers/users/push_subscriptions_controller.rb @@ -5,13 +5,7 @@ class Users::PushSubscriptionsController < ApplicationController end def create - if subscription = @push_subscriptions.find_by(push_subscription_params) - subscription.touch - else - @push_subscriptions.create! push_subscription_params.merge(user_agent: request.user_agent) - end - - head :ok + @push_subscriptions.create_with(user_agent: request.user_agent).create_or_find_by!(push_subscription_params) end def destroy diff --git a/app/models/account.rb b/app/models/account.rb index 61a636adb..0bb4d4111 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -10,8 +10,6 @@ class Account < ApplicationRecord has_many :columns, dependent: :destroy has_many :exports, class_name: "Account::Export", dependent: :destroy - has_many_attached :uploads - before_create :assign_external_account_id after_create :create_join_code diff --git a/app/models/webhook/delivery.rb b/app/models/webhook/delivery.rb index c5dab1497..3ebe49b06 100644 --- a/app/models/webhook/delivery.rb +++ b/app/models/webhook/delivery.rb @@ -1,7 +1,10 @@ class Webhook::Delivery < ApplicationRecord + class ResponseTooLarge < StandardError; end + STALE_TRESHOLD = 7.days USER_AGENT = "fizzy/1.0.0 Webhook" ENDPOINT_TIMEOUT = 7.seconds + MAX_RESPONSE_SIZE = 100.kilobytes belongs_to :account, default: -> { webhook.account } belongs_to :webhook @@ -52,12 +55,16 @@ class Webhook::Delivery < ApplicationRecord if resolved_ip.nil? { error: :private_uri } else - response = http.request( - Net::HTTP::Post.new(uri, headers).tap { |request| request.body = payload } - ) + request = Net::HTTP::Post.new(uri, headers).tap { |request| request.body = payload } + + response = http.request(request) do |net_http_response| + stream_body_with_limit(net_http_response) + end { code: response.code.to_i } end + rescue ResponseTooLarge + { error: :response_too_large } rescue Resolv::ResolvTimeout, Resolv::ResolvError, SocketError { error: :dns_lookup_failed } rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ETIMEDOUT @@ -68,6 +75,14 @@ class Webhook::Delivery < ApplicationRecord { error: :failed_tls } end + def stream_body_with_limit(response) + bytes_read = 0 + response.read_body do |chunk| + bytes_read += chunk.bytesize + raise ResponseTooLarge if bytes_read > MAX_RESPONSE_SIZE + end + end + def resolved_ip return @resolved_ip if defined?(@resolved_ip) @resolved_ip = SsrfProtection.resolve_public_ip(uri.host) diff --git a/app/views/uploads/create.json.jbuilder b/app/views/uploads/create.json.jbuilder deleted file mode 100644 index 984afba49..000000000 --- a/app/views/uploads/create.json.jbuilder +++ /dev/null @@ -1,4 +0,0 @@ -json.message "File uploaded successfully" -json.fileName @upload.filename.to_s -json.mimetype @upload.content_type -json.fileUrl @upload.slug_url diff --git a/config/environments/production.rb b/config/environments/production.rb index 7b29a00d1..86b66b968 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -54,10 +54,10 @@ Rails.application.configure do # Assume all access to the app is happening through a SSL-terminating reverse proxy. # Can be used together with config.force_ssl for Strict-Transport-Security and secure cookies. - config.assume_ssl = true + config.assume_ssl = ENV.fetch("ASSUME_SSL", "true") == "true" # Force all access to the app over SSL, use Strict-Transport-Security, and use secure cookies. - config.force_ssl = true + config.force_ssl = ENV.fetch("FORCE_SSL", "true") == "true" # Log to STDOUT by default config.logger = ActiveSupport::Logger.new(STDOUT) diff --git a/config/initializers/active_storage.rb b/config/initializers/active_storage.rb index 4a4a0343d..74461c795 100644 --- a/config/initializers/active_storage.rb +++ b/config/initializers/active_storage.rb @@ -11,10 +11,9 @@ end # 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 +ActiveSupport.on_load(:active_storage_record) do + configure_replica_connections +end module ActiveStorageControllerExtensions extend ActiveSupport::Concern diff --git a/script/migrations/convert_markdowns_to_rich_text.rb b/script/migrations/convert_markdowns_to_rich_text.rb deleted file mode 100755 index e093730f6..000000000 --- a/script/migrations/convert_markdowns_to_rich_text.rb +++ /dev/null @@ -1,115 +0,0 @@ -#!/usr/bin/env ruby - -require_relative "../config/environment" -require "redcarpet" -require "nokogiri" - -class ActionText::Markdown < ApplicationRecord - belongs_to :record, polymorphic: true -end - -class MarkdownToActionTextConverter - ATTACHMENT_URL_REGEX = %r{/u/(?[^\/\s\)]+)} - - def initialize(html) - @doc = Nokogiri::HTML::DocumentFragment.parse(html) - @attachments = [] - end - - def convert - process_images - process_links - [ @doc.to_html, @attachments ] - end - - private - def process_images - @doc.css("img").each do |img| - src = img["src"].presence - if src && match = src.match(ATTACHMENT_URL_REGEX) - if (attachment = find_attachment(match[:slug])) - img.replace(build_attachment_node(attachment)) - @attachments << attachment - end - end - end - end - - def process_links - @doc.css("a").each do |link| - href = link["href"].presence - - if href && match = href.match(ATTACHMENT_URL_REGEX) - if (attachment = find_attachment(match[:slug])) - link.replace(build_attachment_node(attachment)) - @attachments << attachment - end - end - end - end - - def build_attachment_node(attachment) - html = ActionText::Attachment.from_attachable(attachment).to_html - fragment = Nokogiri::HTML::DocumentFragment.parse(html) - - node = fragment.at_css("action-text-attachment") - node["url"] = Rails.application.routes.url_helpers.rails_blob_path(attachment.blob, only_path: true) - - fragment - end - - def find_attachment(slug) - ActiveStorage::Attachment.find_by(slug: slug) - end -end - -class RedcarpetRenderer - def self.render(markdown) - renderer = Redcarpet::Render::HTML.new - markdowner = Redcarpet::Markdown.new(renderer, - autolink: true, - tables: true, - fenced_code_blocks: true, - strikethrough: true, - superscript: true, - ) - markdowner.render(markdown.to_s) - end -end - -def process_all(klass, field) - klass.find_each do |record| - markdown = ActionText::Markdown.find_by(record: record, name: field) - next unless markdown - - puts "markdown.id=#{markdown.id}" - next unless markdown.record - - html = RedcarpetRenderer.render(markdown.content.to_s) - converter = MarkdownToActionTextConverter.new(html) - rich_text_html, attachments = converter.convert - - rich_text = ActionText::RichText.create!( - name: markdown.name, - record: markdown.record, - body: rich_text_html - ) - - attachments.each do |attachment| - attachment.update!(record: rich_text) - end - - puts "✓ Created rich text for #{markdown.record_type}##{markdown.record_id} (#{markdown.name})" - rescue => e - warn "✗ Failed to process markdown ##{markdown.id}: #{e.class} - #{e.message}" - end -end - -ApplicationRecord.with_each_tenant do |tenant| - puts "Processing tenant: #{tenant}" - - ActionText::RichText.delete_all - - process_all(Card, :description) - process_all(Comment, :body) -end diff --git a/test/controllers/users/push_subscriptions_controller_test.rb b/test/controllers/users/push_subscriptions_controller_test.rb index c41d8313e..4b4dd0a09 100644 --- a/test/controllers/users/push_subscriptions_controller_test.rb +++ b/test/controllers/users/push_subscriptions_controller_test.rb @@ -14,30 +14,12 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest post user_push_subscriptions_path(users(:david)), params: { push_subscription: subscription_params }, headers: { "HTTP_USER_AGENT" => "Mozilla/5.0" } - assert_response :ok + assert_response :no_content assert_equal subscription_params, users(:david).push_subscriptions.last.attributes.slice("endpoint", "p256dh_key", "auth_key") assert_equal "Mozilla/5.0", users(:david).push_subscriptions.last.user_agent end - test "touch existing subscription" do - existing_subscription = users(:david).push_subscriptions.create!( - endpoint: "https://fcm.googleapis.com/fcm/send/abc123", - p256dh_key: "123", - auth_key: "456" - ) - - assert_no_difference -> { users(:david).push_subscriptions.count } do - assert_changes -> { existing_subscription.reload.updated_at } do - post user_push_subscriptions_path(users(:david)), params: { - push_subscription: existing_subscription.attributes.slice("endpoint", "p256dh_key", "auth_key") - } - end - end - - assert_response :ok - end - test "destroy a push subscription" do subscription = users(:david).push_subscriptions.create!( endpoint: "https://fcm.googleapis.com/fcm/send/abc123", diff --git a/test/models/webhook/delivery_test.rb b/test/models/webhook/delivery_test.rb index 30f91390e..d4d95528c 100644 --- a/test/models/webhook/delivery_test.rb +++ b/test/models/webhook/delivery_test.rb @@ -296,12 +296,15 @@ class Webhook::DeliveryTest < ActiveSupport::TestCase stub_dns_resolution(PUBLIC_TEST_IP) # Verify Net::HTTP.new is called with the pinned IP + response_mock = stub(code: "200") + response_mock.stubs(:read_body) + 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")) + http_mock.stubs(:request).yields(response_mock).returns(response_mock) Net::HTTP.expects(:new).with("example.com", 443).returns(http_mock) @@ -310,6 +313,32 @@ class Webhook::DeliveryTest < ActiveSupport::TestCase assert delivery.succeeded? end + test "handles response too large error" do + delivery = webhook_deliveries(:pending) + + large_body = "x" * 200.kilobytes + stub_request(:post, delivery.webhook.url).to_return(status: 200, body: large_body) + + delivery.deliver + + assert_equal "completed", delivery.state + assert_equal "response_too_large", delivery.response[:error] + assert_not delivery.succeeded? + end + + test "allows responses within size limit" do + delivery = webhook_deliveries(:pending) + + small_body = "x" * 50.kilobytes + stub_request(:post, delivery.webhook.url).to_return(status: 200, body: small_body) + + delivery.deliver + + assert_equal "completed", delivery.state + assert_equal 200, delivery.response[:code] + assert delivery.succeeded? + end + private def stub_dns_resolution(*ips) dns_mock = mock("dns")