🐛 Remove internal error details from HTTP error responses (#11288)

* 🚑 Remove internal error details from HTTP error responses

PostgreSQL exceptions, I/O exceptions, and unhandled errors were
leaking raw database messages (table names, constraint names,
SQLSTATE codes), filesystem paths, and internal exception details
to API clients via :hint, :state, and :path response fields.

Remove these fields from server-error responses while keeping
full error context in server-side logs for operators.

Closes #11287

AI-assisted-by: mimo-v2.5-pro

* 🚑 Strip internal fields and map PG errors to safe messages

Complete the security fix for GHSA-r8wx-23q6-w3gf by addressing
the incomplete redaction found in code review.

Add strip-internal-fields helper to dissoc :hint, :state, :path,
and :context from error response data in three handlers that
previously passed raw ex-data through to clients:

- handle-error :internal
- handle-exception :default (else branch)
- handle-error :assertion (else branch)

Add pgsql-state->message to map PostgreSQL SQLSTATE codes to safe,
client-facing messages (e.g. 23505 → "A conflicting entry already
exists") instead of returning raw PG error text. Include :message
in all PSQLException response branches.

Add regression tests asserting :hint, :state, :path, :context are
absent from responses for :internal and unhandled ex-info errors.

Closes #11287

AI-assisted-by: mimo-v2.5-pro

* 🚑 Keep :hint in error protocol, fix unsafe sources

Refine the security fix based on code review feedback.

Keep :hint as part of the error protocol — it is essential for
controlled error communication. Remove it from strip-internal-fields
(which now only strips :state, :path, :context).

Fix the actual sources of unsafe :hint values:

- http/middleware.clj: replace (ex-message cause) with safe static
  strings for IllegalArgumentException, RequestTooBigException, and
  EOFException. These :validation errors return ex-data verbatim
  to clients, so raw exception messages were leaking internals.

- PSQLException handler: use :hint instead of :message for the
  SQLSTATE-mapped messages, staying consistent with the error
  protocol.

Update tests to assert :hint is present (with safe static values)
in :internal and unhandled ex-info responses, and absent only from
bare RuntimeException and IOException responses.

Closes #11287

AI-assisted-by: mimo-v2.5-pro

backend/src/app/http/errors.clj

@@ -34,6 +34,12 @@
         (assoc :request/auth-data (dissoc auth :token))
         (assoc :frontend/version (or (yreq/get-header request "x-frontend-version") "unknown")))))
 
+(defn- strip-internal-fields
+  "Remove fields that leak internal implementation details from error
+  response data. Full context is preserved in server-side logs."
+  [data]
+  (dissoc data :state :path :context))
+
 (defmulti handle-error
   (fn [cause _ _]
     (-> cause ex-data :type)))
@@ -136,6 +142,7 @@
           (l/error :hint "assertion error" :cause cause)
           {::yres/status 500
            ::yres/body   (-> data
+                             (strip-internal-fields)
                              (assoc :type :server-error)
                              (assoc :code :assertion))})))))
 
@@ -161,9 +168,9 @@
       (l/error :hint "internal error" :cause cause)
       {::yres/status 500
        ::yres/body (-> data
+                       (strip-internal-fields)
                        (assoc :type :server-error)
-                       (update :code #(or % :unhandled))
-                       (assoc :hint (ex-message error)))})))
+                       (update :code #(or % :unhandled)))})))
 
 (defmethod handle-error :default
   [error request parent-cause]
@@ -178,6 +185,20 @@
       (handle-exception (:handling edata) request error)
       (handle-exception error request parent-cause))))
 
+(defn- pgsql-state->message
+  "Map PostgreSQL SQLSTATE codes to safe, client-facing messages.
+  Returns a user-friendly string that conveys the nature of the error
+  without exposing table names, constraint names, or other internals."
+  [state]
+  (case state
+    "23505" "A conflicting entry already exists"
+    "23503" "The referenced item does not exist"
+    "23502" "A required field is missing"
+    "23514" "The value violates a data integrity constraint"
+    "57014" "The operation took too long and was cancelled"
+    "25P03" "The transaction was idle too long and was cancelled"
+    "A database error occurred"))
+
 (defmethod handle-exception org.postgresql.util.PSQLException
   [error request parent-cause]
   (let [state (.getSQLState ^java.sql.SQLException error)
@@ -190,20 +211,19 @@
         {::yres/status 504
          ::yres/body {:type :server-error
                       :code :statement-timeout
-                      :hint (ex-message error)}}
+                      :hint (pgsql-state->message state)}}
 
         (= state "25P03")
         {::yres/status 504
          ::yres/body {:type :server-error
                       :code :idle-in-transaction-timeout
-                      :hint (ex-message error)}}
+                      :hint (pgsql-state->message state)}}
 
         :else
         {::yres/status 500
          ::yres/body {:type :server-error
-                      :code :unexpected
-                      :hint (ex-message error)
-                      :state state}}))))
+                      :code :database-error
+                      :hint (pgsql-state->message state)}}))))
 
 (defmethod handle-exception :default
   [error request parent-cause]
@@ -216,17 +236,16 @@
         (l/error :hint "unexpected error" :cause cause)
         {::yres/status 500
          ::yres/body {:type :server-error
-                      :code :unexpected
-                      :hint (ex-message error)}})
+                      :code :unexpected}})
 
       :else
       (binding [l/*context* (request->context request)]
         (l/error :hint "unhandled error" :cause cause)
         {::yres/status 500
          ::yres/body (-> edata
+                         (strip-internal-fields)
                          (assoc :type :server-error)
-                         (update :code #(or % :unhandled))
-                         (assoc :hint (ex-message error)))}))))
+                         (update :code #(or % :unhandled)))}))))
 
 (defmethod handle-exception java.io.IOException
   [cause request _]
@@ -234,9 +253,7 @@
     (l/wrn :hint "io exception" :cause cause)
     {::yres/status 500
      ::yres/body {:type :server-error
-                  :code :io-exception
-                  :hint (ex-message cause)
-                  :path (:path request)}}))
+                  :code :io-exception}}))
 
 (defmethod handle-exception java.util.concurrent.CompletionException
   [cause request _]

backend/src/app/http/middleware.clj

@@ -83,18 +83,18 @@
               (instance? IllegalArgumentException cause)
               (ex/raise :type :validation
                         :code :malformed-json
-                        :hint (ex-message cause)
+                        :hint "invalid JSON in request body"
                         :cause cause)
 
               (instance? RequestTooBigException cause)
               (ex/raise :type :validation
                         :code :request-body-too-large
-                        :hint (ex-message cause))
+                        :hint "request body exceeds size limit")
 
               (instance? java.io.EOFException cause)
               (ex/raise :type :validation
                         :code :malformed-json
-                        :hint (ex-message cause)
+                        :hint "unexpected end of request body"
                         :cause cause)
 
               (instance? RuntimeException cause)

backend/test/backend_tests/http_middleware_test.clj

@@ -6,10 +6,12 @@
 
 (ns backend-tests.http-middleware-test
   (:require
+   [app.common.exceptions :as ex]
    [app.common.time :as ct]
    [app.db :as db]
    [app.http :as-alias http]
    [app.http.access-token]
+   [app.http.errors :as http-errors]
    [app.http.middleware :as mw]
    [app.http.session :as session]
    [app.main :as-alias main]
@@ -300,7 +302,7 @@
     (t/is (instance? clojure.lang.ExceptionInfo ex))
     (t/is (= :validation (-> ex ex-data :type)))
     (t/is (= :malformed-json (-> ex ex-data :code)))
-    (t/is (string? (-> ex ex-data :hint)))))
+    (t/is (= "invalid JSON in request body" (-> ex ex-data :hint)))))
 
 (t/deftest parse-request-request-too-big-exception
   ;; When RequestTooBigException is raised (e.g. the request body
@@ -319,7 +321,7 @@
     (t/is (instance? clojure.lang.ExceptionInfo ex))
     (t/is (= :validation (-> ex ex-data :type)))
     (t/is (= :request-body-too-large (-> ex ex-data :code)))
-    (t/is (string? (-> ex ex-data :hint)))))
+    (t/is (= "request body exceeds size limit" (-> ex ex-data :hint)))))
 
 (t/deftest parse-request-eof-exception
   ;; When java.io.EOFException is raised (e.g. the body stream
@@ -337,7 +339,7 @@
     (t/is (instance? clojure.lang.ExceptionInfo ex))
     (t/is (= :validation (-> ex ex-data :type)))
     (t/is (= :malformed-json (-> ex ex-data :code)))
-    (t/is (string? (-> ex ex-data :hint)))))
+    (t/is (= "unexpected end of request body" (-> ex ex-data :hint)))))
 
 (t/deftest parse-request-runtime-exception-with-cause
   ;; When a RuntimeException with a non-nil ex-cause is raised,
@@ -377,7 +379,7 @@
     (t/is (= 500 (::yres/status response)))
     (t/is (= :server-error (:type body)))
     (t/is (= :unexpected (:code body)))
-    (t/is (= "boom" (:hint body)))))
+    (t/is (nil? (:hint body)))))
 
 (t/deftest parse-request-non-runtime-throwable
   ;; When a non-RuntimeException Throwable is raised (e.g. an
@@ -397,4 +399,45 @@
     (t/is (= 500 (::yres/status response)))
     (t/is (= :server-error (:type body)))
     (t/is (= :io-exception (:code body)))
-    (t/is (= "network gone" (:hint body)))))
+    (t/is (nil? (:hint body)))))
+
+(t/deftest internal-error-strips-sensitive-fields
+  ;; When an :internal error is raised with :state, :path, and
+  ;; :context, those fields must not appear in the response body.
+  ;; :hint is part of the error protocol and is preserved.
+  (let [cause    (ex-info "internal error"
+                          {:type :internal
+                           :code :test-error
+                           :hint "safe user-facing hint"
+                           :state "XX000"
+                           :path "/data/penpot/storage"
+                           :context {:backend :s3 :bucket "prod"}})
+        response (http-errors/handle cause {})
+        body     (::yres/body response)]
+    (t/is (= 500 (::yres/status response)))
+    (t/is (= :server-error (:type body)))
+    (t/is (= :test-error (:code body)))
+    (t/is (= "safe user-facing hint" (:hint body)))
+    (t/is (nil? (:state body)))
+    (t/is (nil? (:path body)))
+    (t/is (nil? (:context body)))))
+
+(t/deftest unhandled-exinfo-strips-sensitive-fields
+  ;; When an ex-info with an unregistered :type (dispatches through
+  ;; handle-exception :default :else) carries :state and :path,
+  ;; those fields must not appear in the response body.
+  ;; :hint is part of the error protocol and is preserved.
+  (let [cause    (ex-info "something broke"
+                          {:type :unregistered-type
+                           :code :custom-code
+                           :hint "safe user-facing hint"
+                           :state "internal-state"
+                           :path "/internal/path"})
+        response (http-errors/handle cause {})
+        body     (::yres/body response)]
+    (t/is (= 500 (::yres/status response)))
+    (t/is (= :server-error (:type body)))
+    (t/is (= :custom-code (:code body)))
+    (t/is (= "safe user-facing hint" (:hint body)))
+    (t/is (nil? (:state body)))
+    (t/is (nil? (:path body)))))