From fb6ece7a7eda9bd3f034aa54f28be3f29f067900 Mon Sep 17 00:00:00 2001 From: Pablo Alba Date: Tue, 8 Sep 2026 13:38:00 +0200 Subject: [PATCH] Revert ":bug: Enforce SSRF checks and add timeouts to HTTP client (#11474)" (#11556) This reverts commit ff63668c1ef61928878cded13bc1c2734c983aff. --- backend/src/app/http/client.clj | 11 +-- backend/src/app/main.clj | 2 +- backend/src/app/media/remote.clj | 6 +- backend/src/app/nitrate.clj | 3 +- .../test/backend_tests/http_client_test.clj | 30 -------- .../test/backend_tests/media_remote_test.clj | 17 ----- .../test/backend_tests/nitrate_ssrf_test.clj | 70 ------------------- 7 files changed, 9 insertions(+), 130 deletions(-) delete mode 100644 backend/test/backend_tests/http_client_test.clj delete mode 100644 backend/test/backend_tests/nitrate_ssrf_test.clj diff --git a/backend/src/app/http/client.clj b/backend/src/app/http/client.clj index 891e581f70..bba77f9aa0 100644 --- a/backend/src/app/http/client.clj +++ b/backend/src/app/http/client.clj @@ -15,7 +15,6 @@ (:require [app.common.schema :as sm] [app.util.ssrf :as ssrf] - [app.worker :as-alias wrk] [cuerdas.core :as str] [integrant.core :as ig] [java-http-clj.core :as http]) @@ -24,8 +23,6 @@ java.net.URI)) (def default-max-redirects 5) -(def default-connect-timeout 30000) -(def default-request-timeout 30000) (defn client? [o] @@ -36,17 +33,15 @@ :pred client?}) (defmethod ig/init-key ::client - [_ {:keys [::wrk/executor]}] - (http/build-client {:connect-timeout default-connect-timeout - :executor executor + [_ _] + (http/build-client {:connect-timeout 30000 :follow-redirects :never})) (defn send! ([client req] (send! client req {})) ([client req {:keys [response-type] :or {response-type :string}}] (assert (client? client) "expected valid http client") - (http/send (merge {:timeout default-request-timeout} req) - {:client client :as response-type}))) + (http/send req {:client client :as response-type}))) (defn- resolve-client [params] diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index cc627f3307..d0ffd1cf58 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -200,7 +200,7 @@ {::db/pool (ig/ref ::db/pool)} ::http.client/client - {::wrk/executor (ig/ref ::wrk/executor)} + {} ::session/manager {::db/pool (ig/ref ::db/pool)} diff --git a/backend/src/app/media/remote.clj b/backend/src/app/media/remote.clj index 9e717b2fd7..0b5a0a4a42 100644 --- a/backend/src/app/media/remote.clj +++ b/backend/src/app/media/remote.clj @@ -75,10 +75,10 @@ {:method method :uri uri :body body - :headers headers - :timeout timeout} + :headers headers} {:response-type :input-stream - :skip-ssrf-check? true}) + :skip-ssrf-check? true + :timeout timeout}) status (:status resp)] (when (not (<= 200 status 299)) (let [body (:body resp)] diff --git a/backend/src/app/nitrate.clj b/backend/src/app/nitrate.clj index 5adafaeced..a189116458 100644 --- a/backend/src/app/nitrate.clj +++ b/backend/src/app/nitrate.clj @@ -68,7 +68,8 @@ "x-profile-id" (str profile-id)} :uri uri :version :http1.1} - (= method :post) (assoc :body (json/encode request-params :key-fn json/write-camel-key)))))) + (= method :post) (assoc :body (json/encode request-params :key-fn json/write-camel-key))) + {:skip-ssrf-check? true}))) (defn- with-retries [handler max-retries] diff --git a/backend/test/backend_tests/http_client_test.clj b/backend/test/backend_tests/http_client_test.clj deleted file mode 100644 index d67edcf182..0000000000 --- a/backend/test/backend_tests/http_client_test.clj +++ /dev/null @@ -1,30 +0,0 @@ -;; This Source Code Form is subject to the terms of the Mozilla Public -;; License, v. 2.0. If a copy of the MPL was not distributed with this -;; file, You can obtain one at http://mozilla.org/MPL/2.0/. -;; -;; Copyright (c) KALEIDOS INC Sucursal en España SL - -(ns backend-tests.http-client-test - (:require - [app.http.client :as http] - [clojure.test :as t] - [java-http-clj.core :as jhttp] - [mockery.core :refer [with-mocks]])) - -(t/deftest send-injects-default-timeout-when-absent - (with-mocks [mock {:target 'java-http-clj.core/send - :return {:status 200 :body ""}}] - (let [client (jhttp/build-client {})] - (http/send! client {:method :get :uri "https://example.com/"}) - (let [[req _opts] (:call-args @mock)] - (t/is (= http/default-request-timeout (:timeout req))))))) - -(t/deftest send-preserves-caller-supplied-timeout - (with-mocks [mock {:target 'java-http-clj.core/send - :return {:status 200 :body ""}}] - (let [client (jhttp/build-client {})] - (http/send! client {:method :get - :uri "https://example.com/" - :timeout 5000}) - (let [[req _opts] (:call-args @mock)] - (t/is (= 5000 (:timeout req))))))) \ No newline at end of file diff --git a/backend/test/backend_tests/media_remote_test.clj b/backend/test/backend_tests/media_remote_test.clj index 21eb46bec8..dfa8b16d05 100644 --- a/backend/test/backend_tests/media_remote_test.clj +++ b/backend/test/backend_tests/media_remote_test.clj @@ -8,7 +8,6 @@ (:require [app.common.exceptions :as ex] [app.config :as cf] - [app.http.client :as http] [app.media.remote :as media.remote] [app.setup :as-alias setup] [app.util.json :as json] @@ -501,22 +500,6 @@ :headers {}})] (t/is (= 200 (:status resp)))))))) -(t/deftest service-request-puts-configured-timeout-in-request - (t/testing "service-request puts media-processing-service-timeout on the http request" - (let [captured (atom nil)] - (with-redefs [cf/get (th/config-get-mock config-mock) - http/req (fn [_client request _opts] - (reset! captured request) - {:status 200 - :body (json-stream {:width 100 :height 100})})] - (media.remote/service-request - (mk-system) - {:method :post - :uri "http://localhost:6065/api/image/info" - :body nil - :headers {}}) - (t/is (= 5000 (:timeout @captured))))))) - ;; --------------------------------------------------------------------------- ;; Shared key ;; --------------------------------------------------------------------------- diff --git a/backend/test/backend_tests/nitrate_ssrf_test.clj b/backend/test/backend_tests/nitrate_ssrf_test.clj deleted file mode 100644 index e44f96356a..0000000000 --- a/backend/test/backend_tests/nitrate_ssrf_test.clj +++ /dev/null @@ -1,70 +0,0 @@ -;; This Source Code Form is subject to the terms of the Mozilla Public -;; License, v. 2.0. If a copy of the MPL was not distributed with this -;; file, You can obtain one at http://mozilla.org/MPL/2.0/. -;; -;; Copyright (c) KALEIDOS INC Sucursal en España SL - -(ns backend-tests.nitrate-ssrf-test - (:require - [app.config :as cf] - [app.http.client :as http] - [app.nitrate :as nitrate] - [app.setup :as-alias setup] - [clojure.string :as str] - [clojure.test :as t] - [integrant.core :as ig] - [java-http-clj.core :as jhttp])) - -(def ^:private private-admin-uri "http://127.0.0.1:9090") - -(defn- mk-cfg - "Minimal nitrate cfg with a real HttpClient and nitrate client methods." - [] - (let [http-client (jhttp/build-client {}) - base {::http/client http-client - ::setup/shared-keys {:admin-console "test-shared-key"}}] - (assoc base ::nitrate/client (ig/init-key ::nitrate/client base)))) - -(defn- with-admin-console-uri - "Run `f` with :admin-console enabled and the given admin-console URI / allowlist." - [admin-uri allowed-hosts f] - (let [original-get cf/get] - (with-redefs [cf/flags #{:admin-console} - cf/get (fn [key & args] - (case key - :admin-console-uri admin-uri - :ssrf-allowed-hosts allowed-hosts - (apply original-get key args)))] - (f)))) - -(t/deftest nitrate-blocks-private-admin-console-uri - (let [sent? (atom false)] - (with-admin-console-uri - private-admin-uri - #{} - (fn [] - (with-redefs [jhttp/send (fn [_req _opts] - (reset! sent? true) - {:status 200 :body "{\"licenses\":true}"})] - (try - (nitrate/call (mk-cfg) :connectivity {}) - (t/is false "should have raised :nitrate-unavailable") - (catch Exception e - (t/is (= :nitrate-unavailable (:type (ex-data e)))) - (t/is (false? @sent?) - "SSRF must stop the request before it reaches the network")))))))) - -(t/deftest nitrate-proceeds-when-admin-console-host-allowlisted - (let [captured (atom nil)] - (with-admin-console-uri - private-admin-uri - #{"127.0.0.1"} - (fn [] - (with-redefs [jhttp/send (fn [req _opts] - (reset! captured req) - {:status 200 - :body "{\"licenses\":true}"})] - (let [result (nitrate/call (mk-cfg) :connectivity {})] - (t/is (= {:licenses true} result)) - (t/is (some? @captured)) - (t/is (str/starts-with? (str (:uri @captured)) private-admin-uri))))))))