From 40301c346305bdd60fbc5f1cf2ecbd866427cf6c Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 4 Jun 2026 13:14:51 +0200 Subject: [PATCH] Make SSRF error message more specific Feedback from devs that were confronted with the "is not an allowed host" message shows, that the message is not very actionable. It's not clear why something that is clearly a legitimate and existing host would be considered "not allowed". The new error message clearly points at the SSRF policy as the source. Making the problem more search engine friendly and hopefully allowing admins to better understand what they have to fix. --- config/locales/en.yml | 2 +- .../app/services/openid_connect/providers/update_service.rb | 2 +- .../services/openid_connect/providers/update_service_spec.rb | 2 +- .../app/validator/nextcloud_compatible_host_validator.rb | 2 +- .../contracts/storages/storages/shared_contract_examples.rb | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 63344b1fc29..a8b9cb507d1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2245,7 +2245,6 @@ en: not_a_datetime: "is not a valid date time." not_a_number: "is not a number." not_allowed: "is invalid because of missing permissions." - host_not_allowed: "is not an allowed host." not_json: "is not parseable as JSON." not_json_object: "is not a JSON object." not_an_integer: "is not an integer." @@ -2258,6 +2257,7 @@ en: regex_list_invalid: "Lines %{invalid_lines} could not be parsed as regular expression." hexcode_invalid: "is not a valid 6-digit hexadecimal color code." smaller_than_or_equal_to_max_length: "must be smaller than or equal to maximum length." + ssrf_filtered: "violates the SSRF policy of this OpenProject instance." taken: "has already been taken." too_long: "is too long (maximum is %{count} characters)." too_short: "is too short (minimum is %{count} characters)." diff --git a/modules/openid_connect/app/services/openid_connect/providers/update_service.rb b/modules/openid_connect/app/services/openid_connect/providers/update_service.rb index 1c4ab6167d4..e29d05de295 100644 --- a/modules/openid_connect/app/services/openid_connect/providers/update_service.rb +++ b/modules/openid_connect/app/services/openid_connect/providers/update_service.rb @@ -114,7 +114,7 @@ module OpenIDConnect if host.present? && OpenProject::SsrfProtection.safe_ip?(host) true else - call.errors.add(:metadata_url, :host_not_allowed) + call.errors.add(:metadata_url, :ssrf_filtered) call.success = false false end diff --git a/modules/openid_connect/spec/services/openid_connect/providers/update_service_spec.rb b/modules/openid_connect/spec/services/openid_connect/providers/update_service_spec.rb index 8ce703e4ae3..7be98aad37f 100644 --- a/modules/openid_connect/spec/services/openid_connect/providers/update_service_spec.rb +++ b/modules/openid_connect/spec/services/openid_connect/providers/update_service_spec.rb @@ -92,7 +92,7 @@ RSpec.describe OpenIDConnect::Providers::UpdateService, type: :model do result = service_call expect(result).not_to be_success - expect(result.errors[:metadata_url]).to include("is not an allowed host.") + expect(result.errors[:metadata_url]).to include("violates the SSRF policy of this OpenProject instance.") expect(httpx_session).not_to have_received(:get) end end diff --git a/modules/storages/app/validator/nextcloud_compatible_host_validator.rb b/modules/storages/app/validator/nextcloud_compatible_host_validator.rb index aabdcbcf612..ef4760e1a38 100644 --- a/modules/storages/app/validator/nextcloud_compatible_host_validator.rb +++ b/modules/storages/app/validator/nextcloud_compatible_host_validator.rb @@ -50,7 +50,7 @@ class NextcloudCompatibleHostValidator < ActiveModel::EachValidator return false if host.blank? return true if OpenProject::SsrfProtection.safe_ip?(host) - contract.errors.add(attribute, :host_not_allowed) + contract.errors.add(attribute, :ssrf_filtered) false rescue URI::InvalidURIError false diff --git a/modules/storages/spec/contracts/storages/storages/shared_contract_examples.rb b/modules/storages/spec/contracts/storages/storages/shared_contract_examples.rb index 0b1f83ae507..d33470a736b 100644 --- a/modules/storages/spec/contracts/storages/storages/shared_contract_examples.rb +++ b/modules/storages/spec/contracts/storages/storages/shared_contract_examples.rb @@ -275,7 +275,7 @@ RSpec.shared_examples_for "nextcloud storage contract", :storage_server_helpers, context "when host is localhost" do let(:storage_host) { "http://localhost:1234" } - include_examples "contract is invalid", host: :host_not_allowed + include_examples "contract is invalid", host: :ssrf_filtered it "does not perform metadata discovery requests" do contract.validate @@ -288,7 +288,7 @@ RSpec.shared_examples_for "nextcloud storage contract", :storage_server_helpers, context "when host uses https protocol" do let(:storage_host) { "https://172.16.193.146" } - include_examples "contract is invalid", host: :host_not_allowed + include_examples "contract is invalid", host: :ssrf_filtered end end