From 8fd679e804749b33e57565387af98b6a3850d55a Mon Sep 17 00:00:00 2001 From: gabrielburnworth Date: Thu, 30 Jul 2026 10:57:12 -0700 Subject: [PATCH 1/8] improve change email form behavior --- app/mutations/users/create.rb | 25 +----- app/mutations/users/email_domain_helpers.rb | 40 ++++++++++ app/mutations/users/update.rb | 3 + .../__tests__/account_settings_test.tsx | 78 +++++++++++++++++-- .../settings/account/account_settings.tsx | 24 ++++-- spec/mutations/users/create_spec.rb | 12 +++ spec/mutations/users/update_spec.rb | 42 ++++++++++ 7 files changed, 190 insertions(+), 34 deletions(-) create mode 100644 app/mutations/users/email_domain_helpers.rb diff --git a/app/mutations/users/create.rb b/app/mutations/users/create.rb index 750d18a1af..339278a79f 100644 --- a/app/mutations/users/create.rb +++ b/app/mutations/users/create.rb @@ -1,9 +1,8 @@ module Users class Create < Mutations::Command include Auth::ConsentHelpers + include Users::EmailDomainHelpers - CANT_USE_SERVER = "You are not authorized to use this server. " \ - "Please use an official email address." ALREADY_REGISTERED = "Already registered" PASSWORD_PROBLEMS = "Password less than 8 characters or does not match " \ "password confirmation." @@ -22,7 +21,7 @@ class Create < Mutations::Command def validate maybe_validate_tos - maybe_check_email_domain + maybe_check_email email.downcase! add_error :email, :*, ALREADY_REGISTERED if User.find_by(email: email) pw_length_ok = password.length > 7 @@ -43,25 +42,5 @@ def execute UserMailer.welcome_email(user).deliver_later unless skip_email { message: "Check your email!" } end - - def allowed_domains - @allowed_domains ||= ENV["TRUSTED_DOMAINS"].split(",").map(&:strip) - end - - def actual_domain - @actual_domain ||= email.split("@").last - end - - def domain_is_ok? - ENV["TRUSTED_DOMAINS"] ? allowed_domains.include?(actual_domain) : true - end - - def you_cant_use_this_server - add_error :email, :email, CANT_USE_SERVER - end - - def maybe_check_email_domain - you_cant_use_this_server unless domain_is_ok? - end end end diff --git a/app/mutations/users/email_domain_helpers.rb b/app/mutations/users/email_domain_helpers.rb new file mode 100644 index 0000000000..94d1928b58 --- /dev/null +++ b/app/mutations/users/email_domain_helpers.rb @@ -0,0 +1,40 @@ +module Users + module EmailDomainHelpers + CANT_USE_SERVER = "You are not authorized to use this server. " \ + "Please use an official email address." + INVALID_EMAIL = "Please enter a valid email address." + + def allowed_domains + @allowed_domains ||= ENV["TRUSTED_DOMAINS"].split(",").map(&:strip) + end + + def actual_domain + @actual_domain ||= email.split("@").last + end + + def domain_is_ok? + ENV["TRUSTED_DOMAINS"] ? allowed_domains.include?(actual_domain) : true + end + + def email_is_ok? + URI::MailTo::EMAIL_REGEXP.match?(email) + end + + def you_cant_use_this_server + add_error :email, :email, CANT_USE_SERVER + end + + def maybe_check_email_domain + you_cant_use_this_server unless domain_is_ok? + end + + def maybe_check_email + unless email_is_ok? + add_error :email, :format, INVALID_EMAIL + return + end + + maybe_check_email_domain + end + end +end diff --git a/app/mutations/users/update.rb b/app/mutations/users/update.rb index 4cafa0e749..d10d0e8c6d 100644 --- a/app/mutations/users/update.rb +++ b/app/mutations/users/update.rb @@ -1,5 +1,7 @@ module Users class Update < Mutations::Command + include Users::EmailDomainHelpers + PASSWORD_PROBLEMS = "Password and confirmation(s) do not match " + "or is less than 8 characters." EMAIL_IN_USE = "That email is already registered" @@ -17,6 +19,7 @@ class Update < Mutations::Command def validate confirm_new_password if password + maybe_check_email if email && attempting_email_change? email_is_invalid = attempting_email_change? && user_already_exists? add_error(:email, :in_use, EMAIL_IN_USE) if email_is_invalid end diff --git a/frontend/settings/account/__tests__/account_settings_test.tsx b/frontend/settings/account/__tests__/account_settings_test.tsx index 8fa1a12a7f..09c13caf8b 100644 --- a/frontend/settings/account/__tests__/account_settings_test.tsx +++ b/frontend/settings/account/__tests__/account_settings_test.tsx @@ -1,6 +1,6 @@ let mockDev = false; import React from "react"; -import { fireEvent, render } from "@testing-library/react"; +import { fireEvent, render, waitFor } from "@testing-library/react"; import { AccountSettings, ActivityBeepSetting, ActivityBeepSettingProps, LandingPageSetting, LandingPageSettingProps, @@ -65,14 +65,17 @@ afterEach(() => { describe("", () => { let requestAccountExportSpy: jest.SpyInstance; + let confirmSpy: jest.SpyInstance; beforeEach(() => { requestAccountExportSpy = jest.spyOn( requestAccountExportModule, "requestAccountExport") .mockImplementation(jest.fn()); + confirmSpy = jest.spyOn(window, "confirm").mockReturnValue(true); }); afterEach(() => { requestAccountExportSpy.mockRestore(); + confirmSpy.mockRestore(); }); const fakeProps = (): AccountSettingsProps => ({ @@ -89,6 +92,7 @@ describe("", () => { const input = container.querySelector(`input[name="${name}"]`); if (!input) { throw new Error(`Expected input for ${name}`); } fireEvent.blur(input, { currentTarget: { value }, target: { value } }); + return input; }; it("changes name", () => { @@ -100,14 +104,76 @@ describe("", () => { expect(saveSpy).toHaveBeenCalledWith(p.user.uuid); }); - it("changes email", () => { + it("changes email", async () => { const p = fakeProps(); - p.user.body.email = ""; + p.user.body.email = "old@example.com"; p.settingsPanelState.account = true; - commitField(p, "email", "new email"); - expect(editSpy).toHaveBeenCalledWith(p.user, { email: "new email" }); + p.dispatch = jest.fn(action => action); + saveSpy.mockImplementation(() => Promise.resolve() as never); + commitField(p, "email", "new@example.com"); + expect(confirmSpy).toHaveBeenCalledWith( + "Change account email address from 'old@example.com' " + + "to 'new@example.com'?"); + expect(editSpy).toHaveBeenCalledWith( + p.user, { email: "new@example.com" }); expect(saveSpy).toHaveBeenCalledWith(p.user.uuid); - expect(success).toHaveBeenCalledWith(Content.CHECK_EMAIL_TO_CONFIRM); + await waitFor(() => + expect(success).toHaveBeenCalledWith(Content.CHECK_EMAIL_TO_CONFIRM)); + }); + + it("rejects an invalid email", () => { + const checkValidity = jest.spyOn( + HTMLInputElement.prototype, "checkValidity").mockReturnValue(false); + const reportValidity = jest.spyOn( + HTMLInputElement.prototype, "reportValidity").mockReturnValue(false); + const p = fakeProps(); + p.user.body.email = "old@example.com"; + p.settingsPanelState.account = true; + const input = commitField(p, "email", "invalid"); + expect(input.getAttribute("type")).toEqual("email"); + expect(checkValidity).toHaveBeenCalled(); + expect(reportValidity).toHaveBeenCalled(); + expect(confirmSpy).not.toHaveBeenCalled(); + expect(editSpy).not.toHaveBeenCalled(); + expect(saveSpy).not.toHaveBeenCalled(); + checkValidity.mockRestore(); + reportValidity.mockRestore(); + }); + + it("does not change email when confirmation is cancelled", () => { + confirmSpy.mockReturnValue(false); + const p = fakeProps(); + p.user.body.email = "old@example.com"; + p.settingsPanelState.account = true; + commitField(p, "email", "new@example.com"); + expect(editSpy).not.toHaveBeenCalled(); + expect(saveSpy).not.toHaveBeenCalled(); + }); + + it("does not change an unchanged email", () => { + const p = fakeProps(); + p.user.body.email = "same@example.com"; + p.settingsPanelState.account = true; + commitField(p, "email", "same@example.com"); + expect(confirmSpy).not.toHaveBeenCalled(); + expect(editSpy).not.toHaveBeenCalled(); + expect(saveSpy).not.toHaveBeenCalled(); + }); + + it("restores the email when saving fails", async () => { + const p = fakeProps(); + p.user.body.email = "old@example.com"; + p.settingsPanelState.account = true; + p.dispatch = jest.fn(action => action); + saveSpy.mockImplementation( + () => Promise.reject(new Error("save failed")) as never); + commitField(p, "email", "new@example.com"); + await waitFor(() => { + expect(editSpy).toHaveBeenLastCalledWith( + p.user, { email: "old@example.com" }); + }); + expect(editSpy).toHaveBeenCalledTimes(2); + expect(success).not.toHaveBeenCalled(); }); it("changes language", () => { diff --git a/frontend/settings/account/account_settings.tsx b/frontend/settings/account/account_settings.tsx index 3d1cb614d4..8399513a93 100644 --- a/frontend/settings/account/account_settings.tsx +++ b/frontend/settings/account/account_settings.tsx @@ -60,14 +60,28 @@ export const AccountSettings = (props: AccountSettingsProps) => {t(DeviceSetting.accountEmail)} { - success(t(Content.CHECK_EMAIL_TO_CONFIRM)); - props.dispatch(edit( - props.user, { email: e.currentTarget.value })); - props.dispatch(save(props.user.uuid)); + const input = e.currentTarget; + const originalEmail = props.user.body.email || ""; + if (!input.checkValidity()) { + input.reportValidity(); + return; + } + if (originalEmail != input.value && confirm( + t("Change account email address from '{{ from }}' to '{{ to }}'?", + { from: originalEmail, to: input.value }, + ))) { + props.dispatch(edit( + props.user, { email: input.value })); + props.dispatch(save(props.user.uuid)) + .then(() => success(t(Content.CHECK_EMAIL_TO_CONFIRM))) + .catch(() => { + props.dispatch(edit(props.user, { email: originalEmail })); + }); + } }} /> diff --git a/spec/mutations/users/create_spec.rb b/spec/mutations/users/create_spec.rb index 8fe82c539c..e61cef7d02 100644 --- a/spec/mutations/users/create_spec.rb +++ b/spec/mutations/users/create_spec.rb @@ -23,6 +23,18 @@ expect(results.success?).to be_truthy end + it "rejects an invalid email address" do + results = Users::Create.run(email: "not-an-email", + name: "Faker", + password: "password12345", + password_confirmation: "password12345", + agree_to_terms: false) + + expect(results.success?).to be false + expect(results.errors.message_list) + .to include(Users::Create::INVALID_EMAIL) + end + it "stops unauthorized users from creating accounts on server" do ClimateControl.modify(TRUSTED_DOMAINS: "farmbot.io,farm.bot") do email = "#{SecureRandom.hex(8)}@qwerty.io" diff --git a/spec/mutations/users/update_spec.rb b/spec/mutations/users/update_spec.rb index b692fdc9a4..c7a6861a23 100644 --- a/spec/mutations/users/update_spec.rb +++ b/spec/mutations/users/update_spec.rb @@ -39,4 +39,46 @@ expect(u.confirmation_token).not_to eq(original_token) end end + + it "rejects an invalid email address" do + user = FactoryBot.create(:user) + result = Users::Update.run(user: user, email: "not-an-email") + + expect(result.success?).to be false + expect(result.errors.message_list) + .to include(Users::Update::INVALID_EMAIL) + end + + it "stops users from changing to an unauthorized email domain" do + user = FactoryBot.create(:user) + + ClimateControl.modify(TRUSTED_DOMAINS: "farmbot.io,farm.bot") do + result = Users::Update.run(user: user, email: "example@mailinator.com") + + expect(result.success?).to be false + expect(result.errors.message_list) + .to include(Users::Update::CANT_USE_SERVER) + end + end + + it "allows users to change to an authorized email domain" do + user = FactoryBot.create(:user) + + ClimateControl.modify(TRUSTED_DOMAINS: "farmbot.io, farm.bot") do + result = Users::Update.run(user: user, email: "example@farm.bot") + + expect(result.success?).to be true + end + end + + it "does not check the domain when no email change is requested" do + email = "#{SecureRandom.hex(8)}@mailinator.com" + user = FactoryBot.create(:user, email: email) + + ClimateControl.modify(TRUSTED_DOMAINS: "farmbot.io") do + result = Users::Update.run(user: user, name: "New Name") + + expect(result.success?).to be true + end + end end From ef8cfba41e41baedd40c6b1458dcb833782c034b Mon Sep 17 00:00:00 2001 From: gabrielburnworth Date: Thu, 30 Jul 2026 10:57:21 -0700 Subject: [PATCH 2/8] restrict tool access --- app/controllers/api/tools_controller.rb | 7 ++++--- app/models/tool.rb | 7 ++++--- .../devices/seeders/demo_account_seeder.rb | 4 +++- app/mutations/tools/destroy.rb | 6 ++++++ app/mutations/tools/update.rb | 13 ++++++++++++- spec/controllers/api/tools/destroy_spec.rb | 6 ++++++ spec/controllers/api/tools/show_spec.rb | 7 +++++++ spec/controllers/api/tools/update_spec.rb | 8 ++++++++ 8 files changed, 50 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/tools_controller.rb b/app/controllers/api/tools_controller.rb index 4abd130b38..5ab8de952f 100644 --- a/app/controllers/api/tools_controller.rb +++ b/app/controllers/api/tools_controller.rb @@ -9,7 +9,7 @@ def show end def destroy - mutate Tools::Destroy.run(tool: tool) + mutate Tools::Destroy.run(tool: tool, device: current_device) end def create @@ -23,7 +23,7 @@ def update private def update_params - output = raw_json.merge(tool: tool) + output = raw_json.merge(tool: tool, device: current_device) output[:name] = params[:name] if params[:name] output end @@ -33,7 +33,8 @@ def tools end def tool - @tool ||= Tool.join_tool_slot_and_find_by_id(params.expect(:id).to_i) + @tool ||= Tool.join_tool_slot_and_find_by_id(params.expect(:id).to_i, + current_device.id) end end end diff --git a/app/models/tool.rb b/app/models/tool.rb index e926798196..95969ad57f 100644 --- a/app/models/tool.rb +++ b/app/models/tool.rb @@ -14,7 +14,7 @@ class Tool < ApplicationRecord "points" ON "points"."tool_id" = "tools"."id" WHERE' INDEX_QUERY = BASE + ' "tools"."device_id" = %s;' - SHOW_QUERY = BASE + ' "tools"."id" = %s;' + SHOW_QUERY = BASE + ' "tools"."id" = %s AND "tools"."device_id" = %s;' IN_USE = "Tool in use by the following sequences: %s" belongs_to :device @@ -26,9 +26,10 @@ def self.outer_join_slots(device_id) self.find_by_sql(INDEX_QUERY % device_id) end - def self.join_tool_slot_and_find_by_id(id) + def self.join_tool_slot_and_find_by_id(id, device_id) # Adding the || self.find part to raise 404 like "normal" AR queries. # TODO: Clean this whole thing up - RC 2-may-18 - self.find_by_sql(SHOW_QUERY % id).first || self.find(id) + self.find_by_sql(format(SHOW_QUERY, id, device_id)).first || + self.where(device_id: device_id).find(id) end end diff --git a/app/mutations/devices/seeders/demo_account_seeder.rb b/app/mutations/devices/seeders/demo_account_seeder.rb index e4f90908c2..dc4efdf81a 100644 --- a/app/mutations/devices/seeders/demo_account_seeder.rb +++ b/app/mutations/devices/seeders/demo_account_seeder.rb @@ -197,7 +197,9 @@ def after_product_line_seeder(product_line) end add_point_groups tool = device.tools.find_by(name: ToolNames::WATERING_NOZZLE) - Tools::Update.run(tool: tool, flow_rate_ml_per_s: 100) if tool + Tools::Update.run(tool: tool, + device: device, + flow_rate_ml_per_s: 100) if tool add_envs marketing_bulletin diff --git a/app/mutations/tools/destroy.rb b/app/mutations/tools/destroy.rb index 752c85550f..88a510b1ca 100644 --- a/app/mutations/tools/destroy.rb +++ b/app/mutations/tools/destroy.rb @@ -10,9 +10,11 @@ class Destroy < Mutations::Command required do model :tool, class: Tool + model :device, class: Device end def validate + validate_ownership any_deps? any_slots? end @@ -24,6 +26,10 @@ def execute private + def validate_ownership + raise Errors::Forbidden unless tool.device_id == device.id + end + # This one will look for other nodes that can possibly reference # a tool's ID, usually within the MARK AS step. # TODO: Write a real SQL view for this. Too busy right now. diff --git a/app/mutations/tools/update.rb b/app/mutations/tools/update.rb index 51a25d89b1..620a0121cd 100644 --- a/app/mutations/tools/update.rb +++ b/app/mutations/tools/update.rb @@ -2,6 +2,7 @@ module Tools class Update < Mutations::Command required do model :tool, class: Tool + model :device, class: Device end optional do @@ -10,8 +11,18 @@ class Update < Mutations::Command float :seeder_tip_z_offset end + def validate + validate_ownership + end + def execute - tool.update!(inputs.except(:tool)) && tool + tool.update!(inputs.except(:tool, :device)) && tool + end + + private + + def validate_ownership + raise Errors::Forbidden unless tool.device_id == device.id end end end diff --git a/spec/controllers/api/tools/destroy_spec.rb b/spec/controllers/api/tools/destroy_spec.rb index 4465424892..dbb33a3890 100644 --- a/spec/controllers/api/tools/destroy_spec.rb +++ b/spec/controllers/api/tools/destroy_spec.rb @@ -75,5 +75,11 @@ expect(before).to eq(after) expect(json[:tool]).to include(Tools::Destroy::STILL_IN_SLOT) end + + it "prevents destruction of another device's tool" do + expect do + Tools::Destroy.run!(tool: tool, device: FactoryBot.create(:device)) + end.to raise_error(Errors::Forbidden) + end end end diff --git a/spec/controllers/api/tools/show_spec.rb b/spec/controllers/api/tools/show_spec.rb index a7e04b40cd..42ff4026ee 100644 --- a/spec/controllers/api/tools/show_spec.rb +++ b/spec/controllers/api/tools/show_spec.rb @@ -15,5 +15,12 @@ expect(response.status).to eq(200) expect(json[:id]).to eq(tool.id) end + + it "does not render another device's tool" do + other_tool = FactoryBot.create(:tool) + sign_in user + get :show, params: { id: other_tool.id } + expect(response.status).to eq(404) + end end end diff --git a/spec/controllers/api/tools/update_spec.rb b/spec/controllers/api/tools/update_spec.rb index 27f1e7f544..3621693458 100644 --- a/spec/controllers/api/tools/update_spec.rb +++ b/spec/controllers/api/tools/update_spec.rb @@ -17,5 +17,13 @@ expect(response.status).to eq(200) expect(tool.reload.name).to eq("Hi!") end + + it "prevents updates to another device's tool" do + expect do + Tools::Update.run!(tool: tool, + device: FactoryBot.create(:device), + name: "Not allowed") + end.to raise_error(Errors::Forbidden) + end end end From 0ef0145ae03b945778369413dcc699a0e445ba39 Mon Sep 17 00:00:00 2001 From: gabrielburnworth Date: Thu, 30 Jul 2026 10:57:32 -0700 Subject: [PATCH 3/8] improve direct image upload feature --- app/controllers/dashboard_controller.rb | 25 ++++++++ app/models/image.rb | 34 +++++++++- app/mutations/images/stub_policy.rb | 5 +- config/routes.rb | 2 + spec/controllers/api/images/images_spec.rb | 48 ++++++++++++-- spec/controllers/dashboard_spec.rb | 73 +++++++++++++++++++++- 6 files changed, 177 insertions(+), 10 deletions(-) diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 1624353e5d..2f261170ea 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -109,9 +109,34 @@ def csp_reports # (for self hosted users) Direct image upload endpoint. # Do not use this if you use GCS- it will slow your app down. def direct_upload + unless Image.valid_direct_upload_token?(key: params[:key], + token: params[:signature]) + render json: { error: "Invalid upload signature" }, status: :unauthorized + return + end + Image.self_hosted_image_upload(key: params.fetch(:key), file: params.fetch(:file)) render json: "" + rescue Image::DirectUploadTooLarge + render json: { error: "Image exceeds maximum size" }, + status: :payload_too_large + rescue Image::InvalidDirectUploadImage + render json: { error: "Upload must be a JPEG image" }, + status: :unsupported_media_type + end + + def direct_upload_file + path = Image.direct_upload_path("#{params.expect(:filename)}.jpg") + unless File.file?(path) + head :not_found + return + end + + response.headers["X-Content-Type-Options"] = "nosniff" + send_file path, + type: "image/jpeg", + disposition: "inline" end def logout; end diff --git a/app/models/image.rb b/app/models/image.rb index 2bf0701b92..ecc6e1f4be 100644 --- a/app/models/image.rb +++ b/app/models/image.rb @@ -1,6 +1,10 @@ require "open-uri" +require "marcel" # A set of image URLs (thumbs) + Associated meta data. class Image < ApplicationRecord + class DirectUploadTooLarge < StandardError; end + class InvalidDirectUploadImage < StandardError; end + belongs_to :device validates :device, presence: true serialize :meta, coder: YAML @@ -23,6 +27,7 @@ def set_defaults x80: "80x80>", } MAX_IMAGE_SIZE = 7.megabytes + DIRECT_UPLOAD_TOKEN_TTL = 1.hour CONFIG = { default_url: DEFAULT_URL, styles: RMAGICK_STYLES, size: { in: 0..MAX_IMAGE_SIZE } } @@ -113,7 +118,34 @@ def self.self_hosted_image_upload(key:, file:) name = key.split("/").last src = file.tempfile.path - dest = File.join("public", "direct_upload", "temp", name) + raise DirectUploadTooLarge if File.size(src) > MAX_IMAGE_SIZE + detected_type = Marcel::MimeType.for(Pathname.new(src)) + raise InvalidDirectUploadImage unless detected_type == "image/jpeg" + + FileUtils.mkdir_p(direct_upload_directory) + dest = direct_upload_path(name) FileUtils.mv(src, dest) end + + def self.direct_upload_path(name) + direct_upload_directory.join(File.basename(name)) + end + + def self.direct_upload_token(key) + direct_upload_verifier.generate(key, expires_in: DIRECT_UPLOAD_TOKEN_TTL) + end + + def self.valid_direct_upload_token?(key:, token:) + token.present? && direct_upload_verifier.verified(token) == key + end + + def self.direct_upload_verifier + Rails.application.message_verifier("direct_upload") + end + private_class_method :direct_upload_verifier + + def self.direct_upload_directory + Rails.root.join("tmp", "direct_upload") + end + private_class_method :direct_upload_directory end diff --git a/app/mutations/images/stub_policy.rb b/app/mutations/images/stub_policy.rb index 26dd4750fd..948d610c2a 100644 --- a/app/mutations/images/stub_policy.rb +++ b/app/mutations/images/stub_policy.rb @@ -3,15 +3,16 @@ class StubPolicy < Mutations::Command URL = "#{$API_URL}/direct_upload/" def execute + key = random_filename { verb: "POST", url: URL, form_data: { - "key" => random_filename, + "key" => key, "acl" => "public-read", "Content-Type" => "image/jpeg", "policy" => "N/A", - "signature" => "N/A", + "signature" => Image.direct_upload_token(key), "GoogleAccessId" => "N/A", "file" => "REPLACE_THIS_WITH_A_BINARY_JPEG_FILE", }, diff --git a/config/routes.rb b/config/routes.rb index 4deb115d26..9d9505614f 100755 --- a/config/routes.rb +++ b/config/routes.rb @@ -138,5 +138,7 @@ get "/verify/:token" => "dashboard#confirmation_page", as: :confirmation_page post "/csp_reports" => "dashboard#csp_reports", as: :csp_report post "/direct_upload" => "dashboard#direct_upload", as: :direct_upload + get "/direct_upload/temp/:filename.jpg" => "dashboard#direct_upload_file", + as: :direct_upload_file post "/webhooks" => "webhooks#create", as: :webhooks end diff --git a/spec/controllers/api/images/images_spec.rb b/spec/controllers/api/images/images_spec.rb index 8979ad7d06..9fb43d1d26 100644 --- a/spec/controllers/api/images/images_spec.rb +++ b/spec/controllers/api/images/images_spec.rb @@ -5,15 +5,18 @@ let(:user) { FactoryBot.create(:user) } it "uploads file" do + tempfile = Tempfile.new(["wow", ".jpg"]) + tempfile.binmode + tempfile.write(File.binread(Rails.root.join("public", "plant.jpg"))) + tempfile.rewind fake_file = ActionDispatch::Http::UploadedFile.new( filename: "wow.jpg", type: "image/jpg", head: "", - tempfile: Tempfile.new, + tempfile: tempfile, ) - name = "wow.jpg" Image.self_hosted_image_upload(key: "/abc.jpg", file: fake_file) - expected = "public/direct_upload/temp/abc.jpg" + expected = Image.direct_upload_path("abc.jpg") begin assert File.file?(expected) ensure @@ -21,6 +24,37 @@ end end + it "rejects oversized direct uploads" do + tempfile = Tempfile.new(["large", ".jpg"]) + tempfile.truncate(Image::MAX_IMAGE_SIZE + 1) + fake_file = ActionDispatch::Http::UploadedFile.new( + filename: "large.jpg", + type: "image/jpeg", + head: "", + tempfile: tempfile, + ) + + expect { + Image.self_hosted_image_upload(key: "/large.jpg", file: fake_file) + }.to raise_error(Image::DirectUploadTooLarge) + end + + it "rejects non-image direct uploads" do + tempfile = Tempfile.new(["fake", ".jpg"]) + tempfile.write("not an image") + tempfile.rewind + fake_file = ActionDispatch::Http::UploadedFile.new( + filename: "fake.jpg", + type: "image/jpeg", + head: "", + tempfile: tempfile, + ) + + expect { + Image.self_hosted_image_upload(key: "/fake.jpg", file: fake_file) + }.to raise_error(Image::InvalidDirectUploadImage) + end + it "Creates a policy object" do allow(Google::Cloud::Storage).to receive_message_chain("new.bucket.post_object.fields") .and_return({ signature: "signature" }) @@ -51,9 +85,15 @@ expect(json).to be_kind_of(Hash) expect(json[:verb]).to eq("POST") expect(json[:url]).to include($API_URL) - [:policy, :signature, :GoogleAccessId] + [:policy, :GoogleAccessId] .map { |key| expect(json.dig(:form_data, key)).to eq("N/A") } expect(json[:form_data].keys.sort).to include(:signature) + expect( + Image.valid_direct_upload_token?( + key: json.dig(:form_data, :key), + token: json.dig(:form_data, :signature), + ), + ).to be(true) end describe "#index" do diff --git a/spec/controllers/dashboard_spec.rb b/spec/controllers/dashboard_spec.rb index 0a85351b67..3db09cf8b2 100644 --- a/spec/controllers/dashboard_spec.rb +++ b/spec/controllers/dashboard_spec.rb @@ -90,13 +90,80 @@ end it "handles self hosted image uploads" do - params = { key: "fake_key", file: "fake_file" } + key = "fake_key" + params = { + key: key, + file: "fake_file", + signature: Image.direct_upload_token(key), + } be_mocked = receive(:self_hosted_image_upload) - .with(params) - .and_return({something: 'testing'}) + .with(key: key, file: "fake_file") + .and_return({ something: "testing" }) expect(Image).to(be_mocked) post :direct_upload, params: params expect(response.status).to eq(200) end + + it "rejects unsigned self hosted image uploads" do + expect(Image).not_to receive(:self_hosted_image_upload) + post :direct_upload, params: { key: "fake_key", file: "fake_file" } + expect(response.status).to eq(401) + end + + it "rejects signatures for a different upload key" do + signature = Image.direct_upload_token("different_key") + expect(Image).not_to receive(:self_hosted_image_upload) + post :direct_upload, params: { + key: "fake_key", + file: "fake_file", + signature: signature, + } + expect(response.status).to eq(401) + end + + it "rejects oversized images" do + key = "fake_key" + allow(Image).to receive(:self_hosted_image_upload) + .and_raise(Image::DirectUploadTooLarge) + post :direct_upload, params: { + key: key, + file: "fake_file", + signature: Image.direct_upload_token(key), + } + expect(response.status).to eq(413) + end + + it "rejects files that are not JPEG images" do + key = "fake_key" + allow(Image).to receive(:self_hosted_image_upload) + .and_raise(Image::InvalidDirectUploadImage) + post :direct_upload, params: { + key: key, + file: "fake_file", + signature: Image.direct_upload_token(key), + } + expect(response.status).to eq(415) + end + + it "serves direct uploads as JPEG images" do + filename = SecureRandom.uuid + path = Image.direct_upload_path("#{filename}.jpg") + FileUtils.mkdir_p(path.dirname) + FileUtils.cp(Rails.root.join("public", "plant.jpg"), path) + + begin + get :direct_upload_file, params: { filename: filename } + expect(response.status).to eq(200) + expect(response.media_type).to eq("image/jpeg") + expect(response.headers["X-Content-Type-Options"]).to eq("nosniff") + ensure + File.delete(path) if File.exist?(path) + end + end + + it "returns not found for missing direct uploads" do + get :direct_upload_file, params: { filename: SecureRandom.uuid } + expect(response.status).to eq(404) + end end end From 921090349e0ac836a257651e1ade81c5a9d112c7 Mon Sep 17 00:00:00 2001 From: gabrielburnworth Date: Thu, 30 Jul 2026 10:57:39 -0700 Subject: [PATCH 4/8] improve sequence unpublish behavior --- .../api/featured_sequences_controller.rb | 5 +--- .../api/sequence_versions_controller.rb | 3 +- app/models/sequence_version.rb | 10 +++++++ .../devices/seeders/abstract_seeder.rb | 3 +- app/mutations/sequences/install.rb | 2 +- app/mutations/sequences/show.rb | 29 +++++++------------ app/mutations/sequences/unpublish.rb | 8 ++++- app/mutations/sequences/upgrade.rb | 2 +- ...7_add_withdrawn_at_to_sequence_versions.rb | 17 +++++++++++ db/structure.sql | 4 ++- .../sequence_versions_show_spec.rb | 21 ++++++++++++++ spec/mutations/sequences/install_spec.rb | 29 +++++++++++++++++++ spec/mutations/sequences/unpublish_spec.rb | 3 ++ spec/mutations/sequences/upgrade_spec.rb | 4 +++ 14 files changed, 111 insertions(+), 29 deletions(-) create mode 100644 db/migrate/20260729150407_add_withdrawn_at_to_sequence_versions.rb diff --git a/app/controllers/api/featured_sequences_controller.rb b/app/controllers/api/featured_sequences_controller.rb index 0182fc3a5d..6b820ba929 100644 --- a/app/controllers/api/featured_sequences_controller.rb +++ b/app/controllers/api/featured_sequences_controller.rb @@ -1,7 +1,5 @@ module Api class FeaturedSequencesController < Api::AbstractController - JOIN = "INNER JOIN sequence_publications ON sequence_publications.id = sequence_versions.sequence_publication_id" - skip_before_action :authenticate_user!, only: [:index] def index @@ -17,9 +15,8 @@ def publisher_email def publications Rails.cache.fetch("farmbot_featured_sequences", expires_in: 10.minutes) do SequenceVersion - .joins(JOIN) + .publicly_available .where(sequence_publications: { cached_author_email: publisher_email }) - .where(sequence_publications: { published: true }) .order(updated_at: :desc) .uniq(&:sequence_publication_id) .map do |x| diff --git a/app/controllers/api/sequence_versions_controller.rb b/app/controllers/api/sequence_versions_controller.rb index 5b91b6b03e..6b1bfdbbd3 100644 --- a/app/controllers/api/sequence_versions_controller.rb +++ b/app/controllers/api/sequence_versions_controller.rb @@ -24,7 +24,8 @@ def version_meta end def sequence_version - @sequence_version ||= SequenceVersion.find(params.expect(:id)) + @sequence_version ||= + SequenceVersion.publicly_available.find(params.expect(:id)) end end end diff --git a/app/models/sequence_version.rb b/app/models/sequence_version.rb index 4b3fb1448b..4f890b361f 100644 --- a/app/models/sequence_version.rb +++ b/app/models/sequence_version.rb @@ -3,11 +3,21 @@ class SequenceVersion < ApplicationRecord belongs_to :sequence_publication has_one :fragment, as: :owner + scope :publicly_available, lambda { + joins(:sequence_publication) + .where(sequence_versions: { withdrawn_at: nil }) + .where(sequence_publications: { published: true }) + } + # We need a #device method on this resource # because Fragment::Create expects it. # it is OK to provide a `nil` device. def device; nil end + def publicly_available? + withdrawn_at.nil? && sequence_publication.published? + end + def broadcast? false end diff --git a/app/mutations/devices/seeders/abstract_seeder.rb b/app/mutations/devices/seeders/abstract_seeder.rb index bf07ee7bc8..6c7cab9a71 100644 --- a/app/mutations/devices/seeders/abstract_seeder.rb +++ b/app/mutations/devices/seeders/abstract_seeder.rb @@ -238,9 +238,8 @@ def tools_rotary; end def install_sequence_version_by_name(name) sv = SequenceVersion - .joins(Api::FeaturedSequencesController::JOIN) + .publicly_available .where(sequence_publications: { cached_author_email: ENV["AUTHORIZED_PUBLISHER"] }) - .where(sequence_publications: { published: true }) .order(updated_at: :desc) .uniq(&:sequence_publication_id) .filter { |x| x.name == name }[0] diff --git a/app/mutations/sequences/install.rb b/app/mutations/sequences/install.rb index f3460b6e54..8e400fa8c0 100644 --- a/app/mutations/sequences/install.rb +++ b/app/mutations/sequences/install.rb @@ -34,7 +34,7 @@ def execute private def validate_publication - unless sequence_version.sequence_publication.published + unless sequence_version.publicly_available? add_error :sequence_version, :version, NOT_PUBLISHED end end diff --git a/app/mutations/sequences/show.rb b/app/mutations/sequences/show.rb index 4a2e52a764..e79e9423ce 100644 --- a/app/mutations/sequences/show.rb +++ b/app/mutations/sequences/show.rb @@ -93,24 +93,17 @@ def is_owner? # Heuristic for determining available sequence version. # def available_version_ids - # First attempt: - # See if the this sequence "owns" is a published upstream publication. - # If it is not published, don't show anything to the author. - # If it IS published, show the versions to the author. - if is_owner? - return sequence_publication&.sequence_versions&.pluck(:id) || [] - end - - # Second attempt: - # The consumer is not the author. - # The sequence has an upstream sequence_version - upstream_sp = sequence_version&.sequence_publication - if upstream_sp&.published - return upstream_sp.sequence_versions.pluck(:id) - end - - # All other cases: Render nothing. - return [] + # Authors get versions from their own publication. Consumers get versions + # from the publication associated with their installed upstream version. + publication = + is_owner? ? sequence_publication : sequence_version&.sequence_publication + + # Unpublished publications expose no versions. Versions withdrawn during + # an earlier unpublish remain usable internally by existing installations, + # but are not offered for preview or upgrade after republishing. + return [] unless publication&.published + + publication.sequence_versions.where(withdrawn_at: nil).pluck(:id) end end end diff --git a/app/mutations/sequences/unpublish.rb b/app/mutations/sequences/unpublish.rb index bd43f4ba1e..7861545c2f 100644 --- a/app/mutations/sequences/unpublish.rb +++ b/app/mutations/sequences/unpublish.rb @@ -12,7 +12,13 @@ def validate end def execute - publication.update!(published: false) + SequencePublication.transaction do + publication + .sequence_versions + .where(withdrawn_at: nil) + .update_all(withdrawn_at: Time.current) + publication.update!(published: false) + end sequence.broadcast!(SecureRandom.uuid) publication end diff --git a/app/mutations/sequences/upgrade.rb b/app/mutations/sequences/upgrade.rb index 96cc90d24f..9aa7da7dbd 100644 --- a/app/mutations/sequences/upgrade.rb +++ b/app/mutations/sequences/upgrade.rb @@ -79,7 +79,7 @@ def validate_ownership end def validate_publication - unless sequence_version.sequence_publication.published + unless sequence_version.publicly_available? add_error :sequence_version, :version, NOT_PUBLISHED end end diff --git a/db/migrate/20260729150407_add_withdrawn_at_to_sequence_versions.rb b/db/migrate/20260729150407_add_withdrawn_at_to_sequence_versions.rb new file mode 100644 index 0000000000..e28de69142 --- /dev/null +++ b/db/migrate/20260729150407_add_withdrawn_at_to_sequence_versions.rb @@ -0,0 +1,17 @@ +class AddWithdrawnAtToSequenceVersions < ActiveRecord::Migration[8.1] + def up + add_column :sequence_versions, :withdrawn_at, :datetime + + execute <<~SQL + UPDATE sequence_versions + SET withdrawn_at = sequence_publications.updated_at + FROM sequence_publications + WHERE sequence_versions.sequence_publication_id = sequence_publications.id + AND sequence_publications.published = FALSE + SQL + end + + def down + remove_column :sequence_versions, :withdrawn_at + end +end diff --git a/db/structure.sql b/db/structure.sql index d743cfbf9f..127d272ae3 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1785,7 +1785,8 @@ CREATE TABLE public.sequence_versions ( color character varying NOT NULL, created_at timestamp(6) without time zone NOT NULL, updated_at timestamp(6) without time zone NOT NULL, - copyright character varying(1500) + copyright character varying(1500), + withdrawn_at timestamp(6) without time zone ); @@ -3851,6 +3852,7 @@ ALTER TABLE ONLY public.users SET search_path TO "$user", public; INSERT INTO "schema_migrations" (version) VALUES +('20260729150407'), ('20260728183708'), ('20260724204047'), ('20260723225144'), diff --git a/spec/controllers/api/sequence_versions/sequence_versions_show_spec.rb b/spec/controllers/api/sequence_versions/sequence_versions_show_spec.rb index 9efcd50ac5..f01721c713 100644 --- a/spec/controllers/api/sequence_versions/sequence_versions_show_spec.rb +++ b/spec/controllers/api/sequence_versions/sequence_versions_show_spec.rb @@ -50,5 +50,26 @@ expect(json[:color]).to eq(sequence.color) expect(json.dig(:body, 0, :comment)).to eq(comment) end + + it "does not show versions withdrawn before republishing" do + publication = Sequences::Publish.run!( + sequence: sequence, + device: author_device, + copyright: "FarmBot, Inc. 2021") + withdrawn_version = publication.sequence_versions.last + + Sequences::Unpublish.run!(device: author_device, sequence: sequence) + republished = Sequences::Publish.run!( + sequence: sequence, + device: author_device, + copyright: "FarmBot, Inc. 2021") + available_version = republished.sequence_versions.last + + get :show, params: { format: :json, id: withdrawn_version.id } + expect(response).to have_http_status(:not_found) + + get :show, params: { format: :json, id: available_version.id } + expect(response).to have_http_status(:ok) + end end end diff --git a/spec/mutations/sequences/install_spec.rb b/spec/mutations/sequences/install_spec.rb index d2ea5a387f..fa7cd470b0 100644 --- a/spec/mutations/sequences/install_spec.rb +++ b/spec/mutations/sequences/install_spec.rb @@ -26,8 +26,37 @@ copyright: "FarmBot, Inc. 2021") publication = Sequences::Unpublish.run!(device: other_device, sequence: pub_seq) sv = publication.sequence_versions.sample + Sequences::Publish.run!(device: other_device, + sequence: pub_seq, + copyright: "FarmBot, Inc. 2021") + expect(publication.reload.published).to be(true) priv_seq = Sequences::Install.run(device: device, sequence_version: sv) msg = "Can't install unpublished sequences" expect(priv_seq.errors["sequence_version"].message).to eq(msg) end + + it "keeps installed versions usable after withdrawal" do + body = [{ kind: "wait", args: { milliseconds: 1000 } }] + pub_seq = FakeSequence.with_parameters(device: other_device, body: body) + publication = Sequences::Publish.run!( + device: other_device, + sequence: pub_seq, + copyright: "FarmBot, Inc.") + installed_version = publication.sequence_versions.last + installed = Sequences::Install.run!( + device: device, + sequence_version: installed_version) + + Sequences::Unpublish.run!(device: other_device, sequence: pub_seq) + republished = Sequences::Publish.run!( + device: other_device, + sequence: pub_seq, + copyright: "FarmBot, Inc.") + available_version = republished.sequence_versions.last + rendered = Sequences::Show.run!(sequence: Sequence.find(installed[:id])) + + expect(rendered.dig(:body, 0, :kind)).to eq("wait") + expect(rendered[:sequence_version_id]).to eq(installed_version.id) + expect(rendered[:sequence_versions]).to eq([available_version.id]) + end end diff --git a/spec/mutations/sequences/unpublish_spec.rb b/spec/mutations/sequences/unpublish_spec.rb index d10727168b..57a462ee52 100644 --- a/spec/mutations/sequences/unpublish_spec.rb +++ b/spec/mutations/sequences/unpublish_spec.rb @@ -10,9 +10,12 @@ publication = Sequences::Publish.run!(sequence: sequence, device: device, copyright: "Farmbot, Inc 2021") + version = publication.sequence_versions.first expect(publication.published).to be(true) + expect(version.withdrawn_at).to be_nil Sequences::Unpublish.run!(device: device, sequence: sequence) expect(publication.reload.published).to be(false) + expect(version.reload.withdrawn_at).to be_present end it "prevents unpublishing other users sequences" do diff --git a/spec/mutations/sequences/upgrade_spec.rb b/spec/mutations/sequences/upgrade_spec.rb index 71456a0714..9a98f768b8 100644 --- a/spec/mutations/sequences/upgrade_spec.rb +++ b/spec/mutations/sequences/upgrade_spec.rb @@ -82,6 +82,10 @@ copyright: "FarmBot, Inc 2021") publication = Sequences::Unpublish.run!(device: other_device, sequence: pub_seq) sv = publication.sequence_versions.sample + Sequences::Publish.run!(device: other_device, + sequence: pub_seq, + copyright: "FarmBot, Inc 2021") + expect(publication.reload.published).to be(true) mut = Sequences::Upgrade.run(device: device, sequence_version: sv, sequence: FakeSequence.with_parameters(device: device)) From b5bc022fc28cda8a73f61927b501888a4a5a9279 Mon Sep 17 00:00:00 2001 From: gabrielburnworth Date: Thu, 30 Jul 2026 10:57:47 -0700 Subject: [PATCH 5/8] improve password reset backend --- app/jobs/password_reset_job.rb | 11 +++ app/mutations/password_resets/create.rb | 22 +---- config/initializers/rack_attack.rb | 81 ++++++++++++++++--- .../password_resets/password_resets_spec.rb | 11 ++- spec/jobs/password_reset_job_spec.rb | 31 +++++++ spec/lib/rack_attack_spec.rb | 73 +++++++++++++++++ 6 files changed, 197 insertions(+), 32 deletions(-) create mode 100644 app/jobs/password_reset_job.rb create mode 100644 spec/jobs/password_reset_job_spec.rb create mode 100644 spec/lib/rack_attack_spec.rb diff --git a/app/jobs/password_reset_job.rb b/app/jobs/password_reset_job.rb new file mode 100644 index 0000000000..261a185b33 --- /dev/null +++ b/app/jobs/password_reset_job.rb @@ -0,0 +1,11 @@ +class PasswordResetJob < ApplicationJob + queue_as :default + + def perform(email) + user = User.find_by(email: email.strip.downcase) + return unless user + + token = PasswordResetToken.issue_to(user).encoded + UserMailer.password_reset(user, token).deliver_later + end +end diff --git a/app/mutations/password_resets/create.rb b/app/mutations/password_resets/create.rb index 77d166953a..e48d4b089b 100644 --- a/app/mutations/password_resets/create.rb +++ b/app/mutations/password_resets/create.rb @@ -4,32 +4,16 @@ class Create < Mutations::Command string :email end - def validate - email_not_found! unless user - end - def execute - send_email + PasswordResetJob.perform_later(normalized_email) # Under no circumstance should you return the token. return { status: "Check your email!" } end private - def send_email - UserMailer.password_reset(user, token).deliver_later - end - - def token - @token ||= PasswordResetToken.issue_to(user).encoded - end - - def email_not_found! - add_error :email, :not_found, "Email not found" - end - - def user - @user ||= User.find_by(email: email) + def normalized_email + @normalized_email ||= email.strip.downcase end end end diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index e5b1cd44c0..039e8f4756 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -1,12 +1,21 @@ +require "json" +require "openssl" + class Rack::Attack + PASSWORD_RESET_BODY_LIMIT = 4096 + PASSWORD_RESET_BODY_THROTTLE = "password_resets/body_size" + PASSWORD_RESET_HMAC_KEY = Rails.application.key_generator + .generate_key("password-reset-rate-limit", 32) + PASSWORD_RESET_IP_LIMIT = 30 + THROTTLE_WARNING = <<~HEREDOC - IP Temporarily Throttled + Request Temporarily Throttled - Your IP address has been throttled due to a high number - of server requests from your web app account or device. + This request has been throttled due to a high number of + similar server requests. - In most cases, your IP address will be unthrottled after - a few minutes. If the problem continues, you may request + In most cases, requests will be allowed again after a few + minutes. If the problem continues, you may request support on the FarmBot forum. Please ensure you are on the latest version of FBOS before requesting support. @@ -36,8 +45,55 @@ class Rack::Attack end end - throttle("password_resets/ip", limit: 3, period: 1.hour) do |req| - req.ip if req.path.downcase == "api/password_resets" + throttle("password_resets/ip", + limit: PASSWORD_RESET_IP_LIMIT, + period: 1.hour) do |req| + req.ip if req.path.downcase == "/api/password_resets" + end + + def self.password_reset_request?(req) + req.post? && req.path.downcase == "/api/password_resets" + end + private_class_method :password_reset_request? + + def self.password_reset_body(req) + return unless password_reset_request?(req) + + body = req.body + body&.read(PASSWORD_RESET_BODY_LIMIT + 1) + ensure + body&.rewind + end + private_class_method :password_reset_body + + throttle(PASSWORD_RESET_BODY_THROTTLE, limit: 0, period: 1.hour) do |req| + body = password_reset_body(req) + req.ip if body && body.bytesize > PASSWORD_RESET_BODY_LIMIT + end + + def self.password_reset_email(req) + body = password_reset_body(req) + return unless body && body.bytesize <= PASSWORD_RESET_BODY_LIMIT + + payload = JSON.parse(body) + email = payload["email"] if payload.is_a?(Hash) + return unless email.is_a?(String) + + normalized_email = email.strip.downcase + return if normalized_email.empty? + + OpenSSL::HMAC.hexdigest( + "SHA256", + PASSWORD_RESET_HMAC_KEY, + normalized_email, + ) + rescue JSON::ParserError + nil + end + private_class_method :password_reset_email + + throttle("password_resets/email", limit: 3, period: 1.hour) do |req| + password_reset_email(req) end end @@ -51,10 +107,15 @@ class Rack::Attack ActiveSupport::Notifications.subscribe("rack.attack") do |_n, _s, _f, _r, req| req = req[:request] if %i[throttle blocklist].include?(req.env["rack.attack.match_type"]) - puts("BLOCKED BY RACK ATTACK: #{req.ip} => #{req.url}") + Rails.logger.warn("BLOCKED BY RACK ATTACK: #{req.ip} => #{req.url}") end end -Rack::Attack.throttled_responder = lambda do |_req| - [429, {}, [Rack::Attack::THROTTLE_WARNING]] +Rack::Attack.throttled_responder = lambda do |req| + matched = req.env["rack.attack.matched"] + if matched == Rack::Attack::PASSWORD_RESET_BODY_THROTTLE + [413, { "content-type" => "text/plain" }, ["Payload Too Large\n"]] + else + [429, {}, [Rack::Attack::THROTTLE_WARNING]] + end end diff --git a/spec/controllers/api/password_resets/password_resets_spec.rb b/spec/controllers/api/password_resets/password_resets_spec.rb index 84a59dca2d..34a3b1abeb 100644 --- a/spec/controllers/api/password_resets/password_resets_spec.rb +++ b/spec/controllers/api/password_resets/password_resets_spec.rb @@ -6,7 +6,7 @@ let(:user) { FactoryBot.create(:user) } it "resets password for a user" do - params = { email: user.email } + params = { email: " #{user.email.upcase} " } old_email_count = ActionMailer::Base.deliveries.length run_jobs_now do @@ -64,9 +64,14 @@ expect(json.to_json).to include(PasswordResets::Update::OLD_TOKEN) end - it "handles bad emails" do + it "does not reveal whether an email is registered" do + expect(PasswordResetJob) + .to receive(:perform_later) + .with("bad@wrong.com") result = PasswordResets::Create.run(email: "bad@wrong.com") - expect(result.errors["email"].message).to eq("Email not found") + + expect(result.success?).to eq(true) + expect(result.result).to eq(status: "Check your email!") end end end diff --git a/spec/jobs/password_reset_job_spec.rb b/spec/jobs/password_reset_job_spec.rb new file mode 100644 index 0000000000..d9207a96b8 --- /dev/null +++ b/spec/jobs/password_reset_job_spec.rb @@ -0,0 +1,31 @@ +require "spec_helper" + +describe PasswordResetJob do + it "sends a password reset email to a registered user" do + user = FactoryBot.create(:user) + + expect { + PasswordResetJob.new.perform(" #{user.email.upcase} ") + }.to change { ActionMailer::Base.deliveries.length }.by(1) + + expect(last_email.to).to include(user.email) + expect(last_email.to_s).to include("password reset") + end + + it "does not send email for an unregistered address" do + expect { + PasswordResetJob.new.perform("unknown@example.com") + }.not_to change { ActionMailer::Base.deliveries.length } + end + + it "delegates delivery to a mailer job" do + user = FactoryBot.create(:user) + token = instance_double(PasswordResetToken, encoded: "encoded-token") + delivery = instance_double(ActionMailer::MessageDelivery) + allow(PasswordResetToken).to receive(:issue_to).and_return(token) + allow(UserMailer).to receive(:password_reset).and_return(delivery) + expect(delivery).to receive(:deliver_later) + + PasswordResetJob.new.perform(user.email) + end +end diff --git a/spec/lib/rack_attack_spec.rb b/spec/lib/rack_attack_spec.rb new file mode 100644 index 0000000000..f6ed2095a5 --- /dev/null +++ b/spec/lib/rack_attack_spec.rb @@ -0,0 +1,73 @@ +require "spec_helper" + +describe Rack::Attack do + around do |example| + original_store = Rack::Attack.cache.store + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + example.run + ensure + Rack::Attack.cache.store = original_store + end + + it "throttles password reset requests by IP" do + app = ->(_env) { [200, {}, ["OK"]] } + request = Rack::MockRequest.new(Rack::Attack.new(app)) + options = { "REMOTE_ADDR" => "192.0.2.1" } + + Rack::Attack::PASSWORD_RESET_IP_LIMIT.times do + expect(request.post("/api/password_resets", options).status).to eq(200) + end + + response = request.post("/api/password_resets", options) + + expect(response.status).to eq(429) + expect(response.body).to eq(Rack::Attack::THROTTLE_WARNING) + end + + it "throttles password resets by normalized email across IP addresses" do + app = ->(_env) { [200, {}, ["OK"]] } + request = Rack::MockRequest.new(Rack::Attack.new(app)) + emails = [ + "target@example.com", + "TARGET@example.com", + " target@example.com ", + "Target@Example.com", + ] + responses = emails.each_with_index.map do |email, index| + request.post( + "/api/password_resets", + "CONTENT_TYPE" => "application/json", + "REMOTE_ADDR" => "192.0.2.#{index + 1}", + input: { email: email }.to_json, + ) + end + + expect(responses.first(3).map(&:status)).to eq([200, 200, 200]) + expect(responses.last.status).to eq(429) + expect(responses.last.body).to eq(Rack::Attack::THROTTLE_WARNING) + end + + it "rejects oversized bodies and counts them against the IP limit" do + app = ->(_env) { [200, {}, ["OK"]] } + request = Rack::MockRequest.new(Rack::Attack.new(app)) + body = { + email: "target@example.com", + padding: "x" * Rack::Attack::PASSWORD_RESET_BODY_LIMIT, + }.to_json + + options = { + "CONTENT_TYPE" => "application/json", + "REMOTE_ADDR" => "192.0.2.1", + input: body, + } + responses = (Rack::Attack::PASSWORD_RESET_IP_LIMIT + 1).times.map do + request.post("/api/password_resets", options) + end + + allowed = responses.first(Rack::Attack::PASSWORD_RESET_IP_LIMIT) + expect(allowed.map(&:status).uniq).to eq([413]) + expect(responses.first.body).to eq("Payload Too Large\n") + expect(responses.last.status).to eq(429) + expect(responses.last.body).to eq(Rack::Attack::THROTTLE_WARNING) + end +end From fea01346e4b1a1e746f47cc9576cfcf032a2d066 Mon Sep 17 00:00:00 2001 From: gabrielburnworth Date: Fri, 31 Jul 2026 10:41:27 -0700 Subject: [PATCH 6/8] add gcs upload key option --- Gemfile | 4 +-- Gemfile.lock | 4 +-- app/mutations/images/generate_policy.rb | 13 ++++++-- example.env | 1 + public/direct_upload/temp/.gitkeep | 0 spec/controllers/api/images/images_spec.rb | 1 + spec/mutations/images/generate_policy_spec.rb | 30 ++++++++++++++++--- 7 files changed, 43 insertions(+), 10 deletions(-) delete mode 100644 public/direct_upload/temp/.gitkeep diff --git a/Gemfile b/Gemfile index da588f989f..9c2b5ce845 100755 --- a/Gemfile +++ b/Gemfile @@ -1,14 +1,14 @@ source "https://rubygems.org" ruby "~> 4.0.6" -gem "rails", "~> 8" +gem "rails" gem "active_model_serializers" gem "bunny" gem "delayed_job_active_record" gem "delayed_job", "4.1.13" gem "devise" gem "discard" -gem "google-cloud-storage", "~> 1.11" +gem "google-cloud-storage" gem "jwt" gem "kaminari" gem "logger" diff --git a/Gemfile.lock b/Gemfile.lock index ca121bf3e9..020482817a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -494,7 +494,7 @@ DEPENDENCIES drb factory_bot_rails faker - google-cloud-storage (~> 1.11) + google-cloud-storage hashdiff irb jwt @@ -511,7 +511,7 @@ DEPENDENCIES rabbitmq_http_api_client rack-attack rack-cors - rails (~> 8) + rails redis (~> 4.0) request_store rollbar diff --git a/app/mutations/images/generate_policy.rb b/app/mutations/images/generate_policy.rb index d5dd201b62..ea002b0dc7 100644 --- a/app/mutations/images/generate_policy.rb +++ b/app/mutations/images/generate_policy.rb @@ -1,5 +1,6 @@ require "google/cloud/storage" require "google/cloud/storage/file" +require "stringio" module Images class GeneratePolicy < Mutations::Command @@ -33,8 +34,16 @@ def bucket_name end def bucket - json_key = ENV["GOOGLE_CLOUD_KEYFILE_JSON"] - json_key && Google::Cloud::Storage.new.bucket(bucket_name) + json_key = ENV["GCS_UPLOAD_KEYFILE_JSON"] + return unless json_key + + credentials = Google::Auth::ServiceAccountCredentials.make_creds( + json_key_io: StringIO.new(json_key), + scope: Google::Cloud::Storage::Credentials::SCOPE, + ) + Google::Cloud::Storage.new( + credentials: credentials, + ).bucket(bucket_name, skip_lookup: true) end def post_object diff --git a/example.env b/example.env index fbe6c27733..db9cb60642 100644 --- a/example.env +++ b/example.env @@ -122,6 +122,7 @@ GCS_ID=GOOGLE_CLOUD_STORAGE='interop' id GCS_KEY=GOOGLE_CLOUD_STORAGE='interop' key GCS_PROJECT= GOOGLE_CLOUD_KEYFILE_JSON= +GCS_UPLOAD_KEYFILE_JSON= # Can be deleted unless you are a Rollbar customer. ROLLBAR_ACCESS_TOKEN=____ diff --git a/public/direct_upload/temp/.gitkeep b/public/direct_upload/temp/.gitkeep deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/spec/controllers/api/images/images_spec.rb b/spec/controllers/api/images/images_spec.rb index 9fb43d1d26..8f8c4da5f4 100644 --- a/spec/controllers/api/images/images_spec.rb +++ b/spec/controllers/api/images/images_spec.rb @@ -61,6 +61,7 @@ with_modified_env( GOOGLE_CLOUD_KEYFILE_JSON: "key", + GCS_UPLOAD_KEYFILE_JSON: "upload-key", GCS_BUCKET: "bucket", ) do diff --git a/spec/mutations/images/generate_policy_spec.rb b/spec/mutations/images/generate_policy_spec.rb index 95d48d92ca..80ce8335ef 100644 --- a/spec/mutations/images/generate_policy_spec.rb +++ b/spec/mutations/images/generate_policy_spec.rb @@ -23,11 +23,7 @@ end it "has a policy object (GCS)" do - allow(Google::Cloud::Storage).to receive(:new) - .and_return(double(bucket: double())) - with_modified_env( - GOOGLE_CLOUD_KEYFILE_JSON: "key", GCS_BUCKET: "gcs", ) do policy = Images::GeneratePolicy.new.send(:policy) @@ -48,4 +44,30 @@ end end end + + it "uses dedicated upload credentials" do + storage = double + bucket = double + credentials = double + + expect(Google::Auth::ServiceAccountCredentials).to receive(:make_creds) + .with( + json_key_io: satisfy { |io| io.read == "upload-key" }, + scope: Google::Cloud::Storage::Credentials::SCOPE, + ) + .and_return(credentials) + expect(Google::Cloud::Storage).to receive(:new).with( + credentials: credentials, + ).and_return(storage) + expect(storage).to receive(:bucket) + .with("bucket", skip_lookup: true) + .and_return(bucket) + + with_modified_env( + GCS_BUCKET: "bucket", + GCS_UPLOAD_KEYFILE_JSON: "upload-key", + ) do + expect(Images::GeneratePolicy.new.send(:bucket)).to eq(bucket) + end + end end From 080f73aa9636e34be6178bc1881681350b7a348c Mon Sep 17 00:00:00 2001 From: gabrielburnworth Date: Fri, 31 Jul 2026 12:11:04 -0700 Subject: [PATCH 7/8] upgrade deps --- Gemfile.lock | 8 ++-- app/controllers/dashboard_controller.rb | 5 ++- app/models/image.rb | 3 +- bun.lock | 42 +++++++++++-------- config/brakeman.ignore | 8 ++++ frontend/ui/__tests__/markdown_test.tsx | 11 +++-- frontend/ui/markdown-it-emoji.d.ts | 1 - frontend/ui/markdown.tsx | 6 +-- package.json | 13 +++--- spec/controllers/api/images/images_spec.rb | 2 + spec/mutations/images/generate_policy_spec.rb | 6 +++ 11 files changed, 63 insertions(+), 42 deletions(-) create mode 100644 config/brakeman.ignore delete mode 100644 frontend/ui/markdown-it-emoji.d.ts diff --git a/Gemfile.lock b/Gemfile.lock index 020482817a..91361c2e69 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -200,7 +200,7 @@ GEM prism (>= 1.3.0) rdoc (>= 4.0.0) reline (>= 0.4.2) - json (2.21.1) + json (2.21.2) jsonapi-renderer (0.2.2) jwt (3.2.0) base64 @@ -354,7 +354,7 @@ GEM zeitwerk (~> 2.6) rainbow (3.1.1) rake (13.4.2) - rbs (4.1.0) + rbs (4.1.1) logger prism (>= 1.6.0) tsort @@ -599,7 +599,7 @@ CHECKSUMS i18n (1.15.2) sha256=00f9eb62412fe593b2a65a97daa75300d37abb8f7202ec748e94b6d46a9dd1b5 io-console (0.8.2) sha256=d6e3ae7a7cc7574f4b8893b4fca2162e57a825b223a177b7afa236c5ef9814cc irb (1.18.0) sha256=de9454a0703a54704b9811a5ef31a60c86949fbf4013fcf244fabc7c775248e3 - json (2.21.1) sha256=13a43df75d95641443f5702dff350f237164a9d811ff0f2c2800d4d980220583 + json (2.21.2) sha256=1f1d3b7cf2b3ba1a69beca0bb6db13d5438b80bff3cd54cdaaa620b9b07c1c6a jsonapi-renderer (0.2.2) sha256=b5c44b033d61b4abdb6500fa4ab84807ca0b36ea0e59e47a2c3ca7095a6e447b jwt (3.2.0) sha256=5419b1fe37b1da0982bd07051f573a8b8789ab724c2aa7e785e4784a3ed217d7 kaminari (1.2.2) sha256=c4076ff9adccc6109408333f87b5c4abbda5e39dc464bd4c66d06d9f73442a3e @@ -667,7 +667,7 @@ CHECKSUMS railties (8.1.3.1) sha256=2388a232579a00cefea4487de66c8553c3408c1300abdc6cf1799d86ffb04487 rainbow (3.1.1) sha256=039491aa3a89f42efa1d6dec2fc4e62ede96eb6acd95e52f1ad581182b79bc6a rake (13.4.2) sha256=cb825b2bd5f1f8e91ca37bddb4b9aaf345551b4731da62949be002fa89283701 - rbs (4.1.0) sha256=8baba59008b0643b4ba2090e9b1d0149655b0bbd42eb2ffe42b1d0eb6923fd72 + rbs (4.1.1) sha256=1bf118f2f95d1cd3956357645d495152b661e0257e57781d18ceecd461622b9e rbtree (0.4.7) sha256=1efabbcb3fd5f12249c9c8a610a765074868164eed0c50c9db2531c00ed161cb rdoc (8.0.0) sha256=03bf8c08a9639658855a0cfd77c0abca8325c227693f7f33f82957811348c469 redis (4.8.1) sha256=387ee086694fffc9632aaeb1efe4a7b1627ca783bf373320346a8a20cd93333a diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 2f261170ea..21248b01f5 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -120,14 +120,15 @@ def direct_upload render json: "" rescue Image::DirectUploadTooLarge render json: { error: "Image exceeds maximum size" }, - status: :payload_too_large + status: :content_too_large rescue Image::InvalidDirectUploadImage render json: { error: "Upload must be a JPEG image" }, status: :unsupported_media_type end def direct_upload_file - path = Image.direct_upload_path("#{params.expect(:filename)}.jpg") + filename = File.basename(params.expect(:filename)) + path = Image.direct_upload_path("#{filename}.jpg") unless File.file?(path) head :not_found return diff --git a/app/models/image.rb b/app/models/image.rb index ecc6e1f4be..4e1baa584b 100644 --- a/app/models/image.rb +++ b/app/models/image.rb @@ -119,6 +119,7 @@ def self.self_hosted_image_upload(key:, file:) name = key.split("/").last src = file.tempfile.path raise DirectUploadTooLarge if File.size(src) > MAX_IMAGE_SIZE + detected_type = Marcel::MimeType.for(Pathname.new(src)) raise InvalidDirectUploadImage unless detected_type == "image/jpeg" @@ -145,7 +146,7 @@ def self.direct_upload_verifier private_class_method :direct_upload_verifier def self.direct_upload_directory - Rails.root.join("tmp", "direct_upload") + Rails.root.join("tmp/direct_upload") end private_class_method :direct_upload_directory end diff --git a/bun.lock b/bun.lock index b7f30c9b41..bb2a051f54 100644 --- a/bun.lock +++ b/bun.lock @@ -10,19 +10,19 @@ "@monaco-editor/react": "4.7.0", "@react-spring/three": "10.1.2", "@react-three/drei": "10.7.7", - "@react-three/fiber": "9.6.1", + "@react-three/fiber": "9.7.0", "@rollbar/react": "1.0.0", "@types/bun": "1.3.14", "@types/lodash": "4.17.24", "@types/markdown-it": "14.1.2", "@types/markdown-it-emoji": "3.0.1", "@types/promise-timeout": "1.3.3", - "@types/react": "19.2.17", + "@types/react": "19.2.18", "@types/react-color": "3.0.13", - "@types/react-dom": "19.2.3", + "@types/react-dom": "19.2.4", "@types/react-test-renderer": "19.1.0", "@types/redux-immutable-state-invariant": "2.1.4", - "@types/three": "0.185.1", + "@types/three": "0.185.3", "@types/ws": "8.18.1", "@typescript/native": "npm:typescript@7.0.2", "@xterm/xterm": "6.0.0", @@ -36,7 +36,7 @@ "fengari-web": "0.1.4", "i18next": "26.3.6", "lodash": "4.18.1", - "markdown-it": "14.3.0", + "markdown-it": "15.0.0", "markdown-it-emoji": "3.1.0", "moment": "2.30.1", "monaco-editor": "0.56.0", @@ -93,7 +93,7 @@ "jshint": "2.13.6", "madge": "8.0.0", "path-browserify": "1.0.1", - "playwright": "1.62.0", + "playwright": "1.62.1", "postcss": "8.5.25", "postcss-scss": "4.0.9", "raf": "3.4.1", @@ -396,7 +396,7 @@ "@react-three/eslint-plugin": ["@react-three/eslint-plugin@0.1.2", "", { "dependencies": { "@babel/runtime": "^7.17.8", "eslint": "^8.12.0" } }, "sha512-jenNIhvt+/1fb3NDr3M5vwF06U9euX6kI2SuAFltVKdQP2nzUPY+zdati2Rd67ewAKn0jfljQWT7DWIe6siChg=="], - "@react-three/fiber": ["@react-three/fiber@9.6.1", "", { "dependencies": { "@babel/runtime": "^7.17.8", "@types/webxr": "*", "base64-js": "^1.5.1", "buffer": "^6.0.3", "its-fine": "^2.0.0", "react-use-measure": "^2.1.7", "scheduler": "^0.27.0", "suspend-react": "^0.1.3", "use-sync-external-store": "^1.4.0", "zustand": "^5.0.3" }, "peerDependencies": { "expo": ">=43.0", "expo-asset": ">=8.4", "expo-file-system": ">=11.0", "expo-gl": ">=11.0", "react": ">=19 <19.3", "react-dom": ">=19 <19.3", "react-native": ">=0.78", "three": ">=0.156" }, "optionalPeers": ["expo", "expo-asset", "expo-file-system", "expo-gl", "react-dom", "react-native"] }, "sha512-zF0rsKcVYpcJwbFEnv2HkHX9cvOEgsfQo/X8lwmR2dn13S4qEQJXir9fxf5js2LQFoXqxOY7MDkOkYx2uZ4gSg=="], + "@react-three/fiber": ["@react-three/fiber@9.7.0", "", { "dependencies": { "@babel/runtime": "^7.17.8", "@types/webxr": "*", "base64-js": "^1.5.1", "buffer": "^6.0.3", "its-fine": "^2.0.0", "react-use-measure": "^2.1.7", "scheduler": "^0.27.0", "suspend-react": "^0.1.3", "use-sync-external-store": "^1.4.0", "zustand": "^5.0.3" }, "peerDependencies": { "expo": ">=43.0", "expo-asset": ">=8.4", "expo-file-system": ">=11.0", "expo-gl": ">=11.0", "react": ">=19 <19.3", "react-dom": ">=19 <19.3", "react-native": ">=0.78", "three": ">=0.156" }, "optionalPeers": ["expo", "expo-asset", "expo-file-system", "expo-gl", "react-dom", "react-native"] }, "sha512-EWm9FwcaOZQu/ExFW5rggoCMM1NJet5YbxVxKaOE+KSncrjU0Wx7017qSyGFvupviK89nMYGCWU3BIK4dI1clw=="], "@rollbar/react": ["@rollbar/react@1.0.0", "", { "dependencies": { "tiny-invariant": "^1.1.0" }, "peerDependencies": { "prop-types": "^15.7.2", "react": "16.x || 17.x || 18.x || 19.x", "rollbar": "^2.26.4 || ^3.0.0-alpha.3" } }, "sha512-e3S9K9k1BLNuqAFA/AD8XH4kcypClYWoMBuW5LO7fnu5Jy1/qiRFIgS5cFZpAFWQqJaSPQAqrLP6n41sH08X9A=="], @@ -488,11 +488,11 @@ "@types/promise-timeout": ["@types/promise-timeout@1.3.3", "", {}, "sha512-gqmIw/4R1F1bqY5hWWZP0YE66iy6KkIu0tICpOLdXBuyHOAaSy9bNvwWHTJxyYHLozkieHM3Ej9GrYA6nuQPMA=="], - "@types/react": ["@types/react@19.2.17", "", { "dependencies": { "csstype": "^3.2.2" } }, "sha512-MXfmqaVPEVgkBT/aY0aGCkRWWtByiYQXo3xdQ8r5RzuFrPiRn8Gar2tQdXSUQ2GKV3bkXckek89V8wQBY2Q/Aw=="], + "@types/react": ["@types/react@19.2.18", "", { "dependencies": { "csstype": "^3.2.2" } }, "sha512-AnzbBERsrLKtk2XSfTbYRLjQPdy116Sty4q+T+Bp3IC4l6jNBvreVPAHmpq9qhXQM7CXZPjLVmGMw9sy+hxQ3w=="], "@types/react-color": ["@types/react-color@3.0.13", "", { "dependencies": { "@types/reactcss": "*" }, "peerDependencies": { "@types/react": "*" } }, "sha512-2c/9FZ4ixC5T3JzN0LP5Cke2Mf0MKOP2Eh0NPDPWmuVH3NjPyhEjqNMQpN1Phr5m74egAy+p2lYNAFrX1z9Yrg=="], - "@types/react-dom": ["@types/react-dom@19.2.3", "", { "peerDependencies": { "@types/react": "^19.2.0" } }, "sha512-jp2L/eY6fn+KgVVQAOqYItbF0VY/YApe5Mz2F0aykSO8gx31bYCZyvSeYxCHKvzHG5eZjc+zyaS5BrBWya2+kQ=="], + "@types/react-dom": ["@types/react-dom@19.2.4", "", { "peerDependencies": { "@types/react": "^19.2.0" } }, "sha512-Bsc+QHgp+P/F02XDzNCY9jnZNCUuLki36KT7VKrTXXLdHf+vHMNZnW1rVu5DNW/rCK+fya3DATySbLM4yhtKUw=="], "@types/react-reconciler": ["@types/react-reconciler@0.28.9", "", { "peerDependencies": { "@types/react": "*" } }, "sha512-HHM3nxyUZ3zAylX8ZEyrDNd2XZOnQ0D5XfunJF5FLQnZbHHYq4UWvW1QfelQNXv1ICNkwYhfxjwfnqivYB6bFg=="], @@ -510,7 +510,7 @@ "@types/suncalc": ["@types/suncalc@1.9.2", "", {}, "sha512-ATAGBHHfA1TlE2tjfidLyTcysjoT2JHHEAmWRULh73SU9UTn++j5fqHEW16X6Y/2Li87jEQXzgu4R/OOdlDqzw=="], - "@types/three": ["@types/three@0.185.1", "", { "dependencies": { "@dimforge/rapier3d-compat": "~0.12.0", "@tweenjs/tween.js": "~23.1.3", "@types/stats.js": "*", "@types/webxr": ">=0.5.17", "fflate": "~0.8.2", "meshoptimizer": "~1.1.1" } }, "sha512-db1xTb+EgYF2didW+eudSvVPtn75zo+fGsY8ShQrJY/B5ZBmC2Fiaykv3aImHAlCNEGuMPkPGXBJGLwzu5mC7A=="], + "@types/three": ["@types/three@0.185.3", "", { "dependencies": { "@dimforge/rapier3d-compat": "~0.12.0", "@tweenjs/tween.js": "~23.1.3", "@types/stats.js": "*", "@types/webxr": ">=0.5.17", "fflate": "~0.8.2", "meshoptimizer": "~1.1.1" } }, "sha512-8TqTn1+fjPWuJ4mR6Igtg56DCf9b5EeAlwhb5xaa6WlBrsg7SvG0NQbyctGRkHCKwg0uftfCspOEsTXelgktMA=="], "@types/tough-cookie": ["@types/tough-cookie@4.0.5", "", {}, "sha512-/Ad8+nIOV7Rl++6f1BdKxFSMgmoqEoYbHRpPcx3JEfv8VRsQe9Z4mCXeJBzxs7mbHY/XOZZuXlRNfhpVPbs6ZA=="], @@ -674,7 +674,7 @@ "app-module-path": ["app-module-path@2.2.0", "", {}, "sha512-gkco+qxENJV+8vFcDiiFhuoSvRXb2a/QPqpSoWhVz829VNJfOTnELbBmPmNKFxf3xdNnw4DWCkzkDaavcX/1YQ=="], - "argparse": ["argparse@2.0.1", "", {}, "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q=="], + "argparse": ["argparse@3.0.0", "", {}, "sha512-BOp5NMrHqKxmq/OLr+clzzrRxgOKSLkcjmkWuChp7Irqwn4s74WjOBPIgWfA/HMcBnVkZ5XEuf9uUqzlpfCQ6A=="], "aria-query": ["aria-query@5.3.0", "", { "dependencies": { "dequal": "^2.0.3" } }, "sha512-b0P0sZPKtyu8HkeRAfCq0IfURZK+SuwMjY1UXGBU27wpAiTwQAIlq56IbIO+ytk/JjS1fMR14ee5WBBfKi5J6A=="], @@ -1536,7 +1536,7 @@ "lines-and-columns": ["lines-and-columns@1.2.4", "", {}, "sha512-7ylylesZQ/PV29jhEDl3Ufjo6ZX7gCqJr5F7PKrqc93v7fzSymt1BpwEU8nAUXs8qzzvqhbjhK5QZg6Mt/HkBg=="], - "linkify-it": ["linkify-it@5.0.2", "", { "dependencies": { "uc.micro": "^2.0.0" } }, "sha512-ONTm2jCMAVZjgQa/Fy1kScXsuOoF5NPTsoFBdE1KVIZ2vAh/r9+Bqo+0jINCBYnavTPQZz38QzFTme79ENoN3Q=="], + "linkify-it": ["linkify-it@6.1.0", "", { "dependencies": { "uc.micro": "^3.0.0" } }, "sha512-wJ/TwpSDTLepCrQoYWYIExIKg5Zchex2Nn5yk2mFnB+6PtdkHtyLx742md9csRjjOnGkKIS/RrbY7l8D6gT9Vw=="], "locate-path": ["locate-path@6.0.0", "", { "dependencies": { "p-locate": "^5.0.0" } }, "sha512-iPZK6eYjbxRu3uB4/WZ3EsEIMJFMqAoopl3R+zuq0UjcAm/MO6KCweDgPfP3elTztoKP3KtnVHxTn2NHBSDVUw=="], @@ -1580,7 +1580,7 @@ "map-visit": ["map-visit@1.0.0", "", { "dependencies": { "object-visit": "^1.0.0" } }, "sha512-4y7uGv8bd2WdM9vpQsiQNo41Ln1NvhvDRuVt0k2JZQ+ezN2uaQes7lZeZ+QQUHOLQAtDaBJ+7wCbi+ab/KFs+w=="], - "markdown-it": ["markdown-it@14.3.0", "", { "dependencies": { "argparse": "^2.0.1", "entities": "^4.5.0", "linkify-it": "^5.0.2", "mdurl": "^2.0.0", "punycode.js": "^2.3.1", "uc.micro": "^2.1.0" }, "bin": { "markdown-it": "bin/markdown-it.mjs" } }, "sha512-RCEsPjR+sr0x+AuYp601tKTkgFG4YEPLCzHST3cQ/fhlJkqAkz1L2/Qbp1j9qw5SBwQHFBoW8+hoN5xssOF0Tw=="], + "markdown-it": ["markdown-it@15.0.0", "", { "dependencies": { "argparse": "^3.0.0", "entities": "^8.0.0", "linkify-it": "^6.0.0", "mdurl": "^2.1.0", "punycode.js": "^2.3.1", "uc.micro": "^3.0.0" }, "bin": { "markdown-it": "bin/markdown-it.mjs" } }, "sha512-Lf8ajvVNdRpzSNB4VegxNy7gjs8gU35l4b4+ET49LrQC5PKYwLZ72u60LeJ9gv3qiaesuYjJWCyVeQmv/QWKQw=="], "markdown-it-emoji": ["markdown-it-emoji@3.1.0", "", {}, "sha512-NhmMEH2ywduD4Nty1E8uB5NqfLhAT1VR0dyvoJyStKOqCzbZmVdn/+8wj7zpDsb/fLBikpCPsWwxqKlvMmbz4g=="], @@ -1594,7 +1594,7 @@ "mdn-data": ["mdn-data@2.27.1", "", {}, "sha512-9Yubnt3e8A0OKwxYSXyhLymGW4sCufcLG6VdiDdUGVkPhpqLxlvP5vl1983gQjJl3tqbrM731mjaZaP68AgosQ=="], - "mdurl": ["mdurl@2.0.0", "", {}, "sha512-Lf+9+2r+Tdp5wXDXC4PcIBjTDtq4UKjCPMQhKIuzpJNW0b96kVqSwW0bT7FhRSfmAiFYgP+SCRvdrDozfh0U5w=="], + "mdurl": ["mdurl@2.1.0", "", {}, "sha512-1+HBaOx0zi/dQWht8rNv9MYf9qqpqL/kxI0hXImU6Y547zM6Sni8BQibt7ifgMcYtQg41ao3Ivd6cnSM86inpg=="], "meow": ["meow@14.1.0", "", {}, "sha512-EDYo6VlmtnumlcBCbh1gLJ//9jvM/ndXHfVXIFrZVr6fGcwTUyCTFNTLCKuY3ffbK8L/+3Mzqnd58RojiZqHVw=="], @@ -1766,9 +1766,9 @@ "pkg-dir": ["pkg-dir@4.2.0", "", { "dependencies": { "find-up": "^4.0.0" } }, "sha512-HRDzbaKjC+AOWVXxAU/x54COGeIv9eb+6CkDSQoNTt4XyWoIJvuPsXizxu/Fr23EiekbtZwmh1IcIG/l/a10GQ=="], - "playwright": ["playwright@1.62.0", "", { "dependencies": { "playwright-core": "1.62.0" }, "optionalDependencies": { "fsevents": "2.3.2" }, "bin": { "playwright": "cli.js" } }, "sha512-Z14dG305dgaLu6foB1TXQagFiW8JfSUIUaUuPaKQ6NtBPKF1P/qXcqfh6c6K/icPqdy37JmjbiBXf6JNg6Sylw=="], + "playwright": ["playwright@1.62.1", "", { "dependencies": { "playwright-core": "1.62.1" }, "optionalDependencies": { "fsevents": "2.3.2" }, "bin": { "playwright": "cli.js" } }, "sha512-0M+L3LAD8/nm554LOla9Ayx0j0tmFZ0FBcoQ7F1VuVHpM/XpiC8RcDzBQB8W5+hA8L22THxELzeF+2WcUzvcLg=="], - "playwright-core": ["playwright-core@1.62.0", "", { "bin": { "playwright-core": "cli.js" } }, "sha512-nsNRyq0r2zsG8AcRHWknc9QRA5XCueC7gWMrs+Gx2tlZn9hcl8zudfh00lhJPY1DE7NmZ6bDsT9g2yey8mXljA=="], + "playwright-core": ["playwright-core@1.62.1", "", { "bin": { "playwright-core": "cli.js" } }, "sha512-wPYSwEBJY9GHraISXqyqtx0na0LpO3XEX7jNDhntbex7tzUS7kLnZsOlFruFJB4Hi/rhDMjXGqHewDZ68nYZVw=="], "pluralize": ["pluralize@8.0.0", "", {}, "sha512-Nc3IT5yHzflTfbjgqWcCPpo7DaKy4FnpB0l/zCAW0Tc7jxAiuqSxHasntB3D7887LSrA93kDJ9IXovxJYxyLCA=="], @@ -2184,7 +2184,7 @@ "typescript": ["typescript@6.0.3", "", { "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" } }, "sha512-y2TvuxSZPDyQakkFRPZHKFm+KKVqIisdg9/CZwm9ftvKXLP8NRWj38/ODjNbr43SsoXqNuAisEf1GdCxqWcdBw=="], - "uc.micro": ["uc.micro@2.1.0", "", {}, "sha512-ARDJmphmdvUk6Glw7y9DQ2bFkKBHwQHLi2lsaH6PPmz/Ka9sFOBsBluozhDltWmnv9u/cF6Rt87znRTPV+yp/A=="], + "uc.micro": ["uc.micro@3.0.0", "", {}, "sha512-U3PppEkleoTnIfi8BozMx3yju3qc/L6SwqWo2Sw+54PX+PX0q9I+r1Um5HCmqD7n9VDX5/v3vQH/AjA6deDdtw=="], "uglify-js": ["uglify-js@3.19.3", "", { "bin": { "uglifyjs": "bin/uglifyjs" } }, "sha512-v3Xu+yuwBXisp6QYTcH4UbH+xYJXqnq2m/LtQVWKWzYc1iehYnLixoQDN9FH6/j9/oybfd6W9Ghwkl8+UMKTKQ=="], @@ -2728,7 +2728,7 @@ "make-dir/semver": ["semver@7.7.3", "", { "bin": "bin/semver.js" }, "sha512-SdsKMrI9TdgjdweUSR9MweHA4EJ8YxHn8DFaDisvhVlUOe4BF1tLD7GAj0lIqWVl+dPb/rExr0Btby5loQm20Q=="], - "markdown-it/entities": ["entities@4.5.0", "", {}, "sha512-V0hjH4dGPh9Ao5p0MoRY6BVqtwCjhz6vI5LT8AJ55H+4g9/4vbHx1I54fS0XuclLhDHArPQCiMjDxjaL8fPxhw=="], + "markdown-it/entities": ["entities@8.0.0", "", {}, "sha512-zwfzJecQ/Uej6tusMqwAqU/6KL2XaB2VZ2Jg54Je6ahNBGNH6Ek6g3jjNCF0fG9EWQKGZNddNjU5F1ZQn/sBnA=="], "micromatch/picomatch": ["picomatch@2.3.1", "", {}, "sha512-JU3teHTNjmE2VCGFzuY8EXzCDVwEqB2a8fsIvwaStHhAWJEeVd1o1QD80CU6+ZdEXXSLbSsuLwJjkCBWqRQUVA=="], @@ -2918,6 +2918,8 @@ "@eslint/eslintrc/espree/eslint-visitor-keys": ["eslint-visitor-keys@3.4.3", "", {}, "sha512-wpc+LXeiyiisxPlEkUzU6svyS1frIO3Mgxj1fdy7Pm8Ygzguax2N3Fa/D/ag1WqbOprdI+uY6wMUl8/a2G+iag=="], + "@eslint/eslintrc/js-yaml/argparse": ["argparse@2.0.1", "", {}, "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q=="], + "@eslint/eslintrc/minimatch/brace-expansion": ["brace-expansion@1.1.11", "", { "dependencies": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" } }, "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA=="], "@humanwhocodes/config-array/minimatch/brace-expansion": ["brace-expansion@1.1.11", "", { "dependencies": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" } }, "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA=="], @@ -3006,6 +3008,8 @@ "class-utils/define-property/is-descriptor": ["is-descriptor@0.1.7", "", { "dependencies": { "is-accessor-descriptor": "^1.0.1", "is-data-descriptor": "^1.0.1" } }, "sha512-C3grZTvObeN1xud4cRWl366OMXZTj0+HGyk4hvfpx4ZHt1Pb60ANSXqCK7pdOTeUQpRzECBSTphqvD7U+l22Eg=="], + "cosmiconfig/js-yaml/argparse": ["argparse@2.0.1", "", {}, "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q=="], + "detective-typescript/@typescript-eslint/typescript-estree/@typescript-eslint/types": ["@typescript-eslint/types@7.15.0", "", {}, "sha512-aV1+B1+ySXbQH0pLK0rx66I3IkiZNidYobyfn0WFsdGhSXw+P3YOqeTq5GED458SfB24tg+ux3S+9g118hjlTw=="], "detective-typescript/@typescript-eslint/typescript-estree/@typescript-eslint/visitor-keys": ["@typescript-eslint/visitor-keys@7.15.0", "", { "dependencies": { "@typescript-eslint/types": "7.15.0", "eslint-visitor-keys": "^3.4.3" } }, "sha512-Hqgy/ETgpt2L5xueA/zHHIl4fJI2O4XUE9l4+OIfbJIRSnTJb/QscncdqqZzofQegIJugRIF57OJea1khw2SDw=="], @@ -3258,6 +3262,8 @@ "@react-three/eslint-plugin/eslint/file-entry-cache/flat-cache": ["flat-cache@3.2.0", "", { "dependencies": { "flatted": "^3.2.9", "keyv": "^4.5.3", "rimraf": "^3.0.2" } }, "sha512-CYcENa+FtcUKLmhhqyctpclsq7QF38pKjZHsGNiSQF5r4FtoKDWabFDl3hzaEQMvT1LHEysw5twgLvpYYb4vbw=="], + "@react-three/eslint-plugin/eslint/js-yaml/argparse": ["argparse@2.0.1", "", {}, "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q=="], + "@react-three/eslint-plugin/eslint/minimatch/brace-expansion": ["brace-expansion@1.1.11", "", { "dependencies": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" } }, "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA=="], "@vue/compiler-core/@babel/parser/@babel/types/@babel/helper-string-parser": ["@babel/helper-string-parser@7.25.9", "", {}, "sha512-4A/SCr/2KLd5jrtOMFzaKjVtAei3+2r/NChoBNoZ3EyP/+GlhoaEGoWOZUmFmoITP7zOJyHIMm+DYRd8o3PvHA=="], diff --git a/config/brakeman.ignore b/config/brakeman.ignore new file mode 100644 index 0000000000..adc570d529 --- /dev/null +++ b/config/brakeman.ignore @@ -0,0 +1,8 @@ +{ + "ignored_warnings": [ + { + "fingerprint": "6eb5910f38bccb50a3b99a622eec8d919df4605274b6c27e960356220ba5a396", + "note": "sanitized with File.basename" + } + ] +} diff --git a/frontend/ui/__tests__/markdown_test.tsx b/frontend/ui/__tests__/markdown_test.tsx index 4f26c71c77..3677bef74b 100644 --- a/frontend/ui/__tests__/markdown_test.tsx +++ b/frontend/ui/__tests__/markdown_test.tsx @@ -1,6 +1,5 @@ import React from "react"; -import type Renderer from "markdown-it/lib/renderer.mjs"; -import type Token from "markdown-it/lib/token.mjs"; +import type { Renderer, Token } from "markdown-it"; import { render } from "@testing-library/react"; import { Markdown, md_for_tests } from "../markdown"; @@ -31,18 +30,18 @@ describe("link_open_for_tests()", () => { const tokens = fakeTokens(); tokens[0].attrIndex = () => -1; const { renderToken, self } = fakeRenderer(); - md_for_tests.renderer.rules.link_open?.(tokens, 0, {}, {}, + md_for_tests.renderer.rules.link_open?.(tokens, 0, md_for_tests.options, {}, self); expect(tokens[0].attrPush).toHaveBeenCalledWith(["target", "_blank"]); - expect(renderToken).toHaveBeenCalledWith(tokens, 0, {}); + expect(renderToken).toHaveBeenCalledWith(tokens, 0, md_for_tests.options); }); it("updates attribute", () => { const tokens = fakeTokens(); const { renderToken, self } = fakeRenderer(); - md_for_tests.renderer.rules.link_open?.(tokens, 0, {}, {}, + md_for_tests.renderer.rules.link_open?.(tokens, 0, md_for_tests.options, {}, self); expect(tokens[0].attrs?.[0][1]).toEqual("_blank"); - expect(renderToken).toHaveBeenCalledWith(tokens, 0, {}); + expect(renderToken).toHaveBeenCalledWith(tokens, 0, md_for_tests.options); }); }); diff --git a/frontend/ui/markdown-it-emoji.d.ts b/frontend/ui/markdown-it-emoji.d.ts deleted file mode 100644 index e1a0be37e4..0000000000 --- a/frontend/ui/markdown-it-emoji.d.ts +++ /dev/null @@ -1 +0,0 @@ -declare module "markdown-it-emoji"; diff --git a/frontend/ui/markdown.tsx b/frontend/ui/markdown.tsx index 24507bc987..98185bbd5e 100644 --- a/frontend/ui/markdown.tsx +++ b/frontend/ui/markdown.tsx @@ -1,13 +1,13 @@ import React from "react"; import { full as emoji } from "markdown-it-emoji"; -import markdownit, { PluginSimple } from "markdown-it"; +import markdownit from "markdown-it"; const md = markdownit({ breaks: true, linkify: true, typographer: true, }) - .use(emoji as PluginSimple); + .use(emoji); const md_with_html = markdownit({ /** Enable HTML tags in source */ @@ -19,7 +19,7 @@ const md_with_html = markdownit({ /** Enable some language-neutral replacement + quotes beautification */ typographer: true, }) - .use(emoji as PluginSimple); + .use(emoji); const defaultRenderer = md.renderer.rules.link_open || // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/package.json b/package.json index 2499cf3602..0de45d834c 100644 --- a/package.json +++ b/package.json @@ -45,19 +45,18 @@ "@monaco-editor/react": "4.7.0", "@react-spring/three": "10.1.2", "@react-three/drei": "10.7.7", - "@react-three/fiber": "9.6.1", + "@react-three/fiber": "9.7.0", "@rollbar/react": "1.0.0", "@types/bun": "1.3.14", "@types/lodash": "4.17.24", - "@types/markdown-it": "14.1.2", "@types/markdown-it-emoji": "3.0.1", "@types/promise-timeout": "1.3.3", - "@types/react": "19.2.17", + "@types/react": "19.2.18", "@types/react-color": "3.0.13", - "@types/react-dom": "19.2.3", + "@types/react-dom": "19.2.4", "@types/react-test-renderer": "19.1.0", "@types/redux-immutable-state-invariant": "2.1.4", - "@types/three": "0.185.1", + "@types/three": "0.185.3", "@types/ws": "8.18.1", "@typescript/native": "npm:typescript@7.0.2", "@xterm/xterm": "6.0.0", @@ -71,7 +70,7 @@ "fengari-web": "0.1.4", "i18next": "26.3.6", "lodash": "4.18.1", - "markdown-it": "14.3.0", + "markdown-it": "15.0.0", "markdown-it-emoji": "3.1.0", "moment": "2.30.1", "monaco-editor": "0.56.0", @@ -128,7 +127,7 @@ "jshint": "2.13.6", "madge": "8.0.0", "path-browserify": "1.0.1", - "playwright": "1.62.0", + "playwright": "1.62.1", "postcss": "8.5.25", "postcss-scss": "4.0.9", "raf": "3.4.1", diff --git a/spec/controllers/api/images/images_spec.rb b/spec/controllers/api/images/images_spec.rb index 8f8c4da5f4..8341d7c4ef 100644 --- a/spec/controllers/api/images/images_spec.rb +++ b/spec/controllers/api/images/images_spec.rb @@ -56,6 +56,8 @@ end it "Creates a policy object" do + allow(Google::Auth::ServiceAccountCredentials).to receive(:make_creds) + .and_return(double) allow(Google::Cloud::Storage).to receive_message_chain("new.bucket.post_object.fields") .and_return({ signature: "signature" }) diff --git a/spec/mutations/images/generate_policy_spec.rb b/spec/mutations/images/generate_policy_spec.rb index 80ce8335ef..a7d52faa64 100644 --- a/spec/mutations/images/generate_policy_spec.rb +++ b/spec/mutations/images/generate_policy_spec.rb @@ -70,4 +70,10 @@ expect(Images::GeneratePolicy.new.send(:bucket)).to eq(bucket) end end + + it "does not create a bucket without upload credentials" do + with_modified_env(GCS_UPLOAD_KEYFILE_JSON: nil) do + expect(Images::GeneratePolicy.new.send(:bucket)).to be_nil + end + end end From 75b1bdfeed7dcddbb01f6fd158e23bc207defdf7 Mon Sep 17 00:00:00 2001 From: gabrielburnworth Date: Fri, 31 Jul 2026 14:41:05 -0700 Subject: [PATCH 8/8] update bun.lock --- bun.lock | 1 - 1 file changed, 1 deletion(-) diff --git a/bun.lock b/bun.lock index bb2a051f54..fbbc3cb335 100644 --- a/bun.lock +++ b/bun.lock @@ -14,7 +14,6 @@ "@rollbar/react": "1.0.0", "@types/bun": "1.3.14", "@types/lodash": "4.17.24", - "@types/markdown-it": "14.1.2", "@types/markdown-it-emoji": "3.0.1", "@types/promise-timeout": "1.3.3", "@types/react": "19.2.18",