bterkuile/fizzy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'main' into mobile-app/scoped-stylesheets

Browse Source 
* main:
  Stream response in webhook request manually to check size
  Fix stale-read race when creating push subscriptions
  Drop defunct Account upload attachments (#2030)
  Process blob variants using local files
  Allow configuring SSL in production with ENV vars
This commit is contained in:
Adrien Maston
2025-12-09 12:47:32 +01:00
parent 072089a9ce da2bd2f2ec
commit 37f41d1152
12 changed files with 78 additions and 169 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gemfile
+3 -2
View File
@@ -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
Gemfile.lock
+10 -5
View File
@@ -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
Gemfile.saas.lock
+10 -5
View File
@@ -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
app/controllers/users/push_subscriptions_controller.rb
+1 -7
View File
@@ -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
app/models/account.rb
-2
View File
@@ -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
app/models/webhook/delivery.rb
+18 -3
View File
@@ -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)
app/views/uploads/create.json.jbuilder
-4
View File
@@ -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
config/environments/production.rb
+2 -2
View File
@@ -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)
config/initializers/active_storage.rb
+3 -4
View File
@@ -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
script/migrations/convert_markdowns_to_rich_text.rb
-115
View File
@@ -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/(?<slug>[^\/\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
test/controllers/users/push_subscriptions_controller_test.rb
+1 -19
View File
@@ -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",
test/models/webhook/delivery_test.rb
+30 -1
View File
@@ -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")