From 9e52bb40d080184e1ce0126448d8573922a6c807 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 19 Jun 2026 11:30:48 +0200 Subject: [PATCH] :sparkles: Add process-level resource limits to font processing tools (#10274) * :sparkles: Add font processing resource limits via prlimit Font processing tools (fontforge, sfnt2woff, woff2sfnt, woff2_decompress) were invoked via clojure.java.shell/sh with no timeouts or resource limits. This adds process-level resource limits using prlimit(1) and the shell/exec! infrastructure from the ImageMagick hardening work. shell/exec! changes: - Add :prlimit parameter that prepends prlimit(1) to the command - :prlimit takes {:mem :cpu } for address space and CPU time limits, enforced by the kernel's RLIMIT subsystem - prlimit-cmd builds the prlimit command prefix (private helper) Font processing changes: - Replace all clojure.java.shell/sh calls with shell/exec! via exec-font! - exec-font! applies font-prlimit (512 MiB, 30s CPU, 60s wall-clock) - All 5 conversion functions (ttf->otf, otf->ttf, ttf-or-otf->woff, woff->sfnt, woff2->sfnt) use try/finally for explicit temp file cleanup - Remove clojure.java.shell require from media.clj Tests: - Add exec-prlimit-normal, exec-prlimit-cpu, exec-prlimit-memory tests Closes #10234 Co-authored-by: mimo-v2.5-pro * :sparkles: Make font processing resource limits configurable Replace hardcoded font-prlimit map and wall-clock timeout with config-driven values under the PENPOT_FONT_PROCESS_* namespace. The prlimit implementation detail is not exposed in config keys. Co-authored-by: deepseek-v4-flash --------- Co-authored-by: mimo-v2.5-pro Co-authored-by: deepseek-v4-flash --- backend/src/app/config.clj | 9 +++ backend/src/app/media.clj | 97 +++++++++++++++-------- backend/src/app/util/shell.clj | 20 ++++- backend/test/backend_tests/shell_test.clj | 29 +++++++ 4 files changed, 120 insertions(+), 35 deletions(-) diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 91c3f57506..3fcdfd6443 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -74,6 +74,10 @@ :media-max-file-size (* 1024 1024 30) ; 30MiB :font-max-file-size (* 1024 1024 30) ; 30MiB + :font-process-mem 512 ;; 512 MiB address space ceiling + :font-process-cpu 30 ;; 30 seconds CPU time + :font-process-timeout 60 ;; 60 seconds wall-clock + :ldap-user-query "(|(uid=:username)(mail=:username))" :ldap-attrs-username "uid" :ldap-attrs-email "mail" @@ -128,6 +132,11 @@ [:media-max-file-size {:optional true} ::sm/int] [:font-max-file-size {:optional true} ::sm/int] + ;; Font processing resource limits (PENPOT_FONT_PROCESS_*) + [:font-process-mem {:optional true} ::sm/int] + [:font-process-cpu {:optional true} ::sm/int] + [:font-process-timeout {:optional true} ::sm/int] + ;; ImageMagick resource limits (PENPOT_IMAGEMAGICK_*) [:imagemagick-thread-limit {:optional true} :string] [:imagemagick-memory-limit {:optional true} :string] diff --git a/backend/src/app/media.clj b/backend/src/app/media.clj index 4a1143ee4b..d3ff7fdff2 100644 --- a/backend/src/app/media.clj +++ b/backend/src/app/media.clj @@ -24,7 +24,6 @@ [app.util.shell :as shell] [buddy.core.bytes :as bb] [buddy.core.codecs :as bc] - [clojure.java.shell :as sh] [clojure.string] [clojure.xml :as xml] [cuerdas.core :as str] @@ -389,48 +388,80 @@ ;; FONTS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +(defn- get-font-prlimit + "Returns resource limits for font processing tools, read from config." + [] + {:mem (cf/get :font-process-mem) + :cpu (cf/get :font-process-cpu)}) + +(defn- get-font-timeout + "Returns the wall-clock timeout for font processing, read from config." + [] + (cf/get :font-process-timeout)) + +(defn- exec-font! + "Execute a font processing command with resource limits. + `args` is a vector of string arguments." + [system args] + (shell/exec! system + :cmd args + :prlimit (get-font-prlimit) + :timeout (get-font-timeout))) + (defmethod process :generate-fonts - [_system {:keys [input] :as params}] + [system {:keys [input] :as params}] (letfn [(ttf->otf [data] (let [finput (tmp/tempfile :prefix "penpot.font." :suffix "") - foutput (fs/path (str finput ".otf")) - _ (io/write* finput data) - res (sh/sh "fontforge" "-lang=ff" "-c" - (str/fmt "Open('%s'); Generate('%s')" - (str finput) - (str foutput)))] - (when (zero? (:exit res)) - foutput))) + foutput (fs/path (str finput ".otf"))] + (try + (io/write* finput data) + (let [res (exec-font! system ["fontforge" "-lang=ff" "-c" + (str/fmt "Open('%s'); Generate('%s')" + (str finput) + (str foutput))])] + (when (zero? (:exit res)) + foutput)) + (finally + (fs/delete finput))))) (otf->ttf [data] (let [finput (tmp/tempfile :prefix "penpot.font." :suffix "") - foutput (fs/path (str finput ".ttf")) - _ (io/write* finput data) - res (sh/sh "fontforge" "-lang=ff" "-c" - (str/fmt "Open('%s'); Generate('%s')" - (str finput) - (str foutput)))] - (when (zero? (:exit res)) - foutput))) + foutput (fs/path (str finput ".ttf"))] + (try + (io/write* finput data) + (let [res (exec-font! system ["fontforge" "-lang=ff" "-c" + (str/fmt "Open('%s'); Generate('%s')" + (str finput) + (str foutput))])] + (when (zero? (:exit res)) + foutput)) + (finally + (fs/delete finput))))) (ttf-or-otf->woff [data] - ;; NOTE: foutput is not used directly, it represents the - ;; default output of the execution of the underlying - ;; command. (let [finput (tmp/tempfile :prefix "penpot.font." :suffix "") - foutput (fs/path (str finput ".woff")) - _ (io/write* finput data) - res (sh/sh "sfnt2woff" (str finput))] - (when (zero? (:exit res)) - foutput))) + foutput (fs/path (str finput ".woff"))] + (try + (io/write* finput data) + (let [res (exec-font! system ["sfnt2woff" (str finput)])] + (when (zero? (:exit res)) + foutput)) + (finally + (fs/delete finput))))) (woff->sfnt [data] - (let [finput (tmp/tempfile :prefix "penpot" :suffix "") - _ (io/write* finput data) - res (sh/sh "woff2sfnt" (str finput) - :out-enc :bytes)] - (when (zero? (:exit res)) - (:out res)))) + (let [finput (tmp/tempfile :prefix "penpot" :suffix "")] + (try + (io/write* finput data) + (let [res (shell/exec! system + :cmd ["woff2sfnt" (str finput)] + :out-enc :bytes + :prlimit (get-font-prlimit) + :timeout (get-font-timeout))] + (when (zero? (:exit res)) + (:out res))) + (finally + (fs/delete finput))))) (woff2->sfnt [data] ;; woff2_decompress outputs to same directory with .ttf extension @@ -438,7 +469,7 @@ foutput (fs/path (str/replace (str finput) #"\.woff2$" ".ttf"))] (try (io/write* finput data) - (let [res (sh/sh "woff2_decompress" (str finput))] + (let [res (exec-font! system ["woff2_decompress" (str finput)])] (if (zero? (:exit res)) foutput (do diff --git a/backend/src/app/util/shell.clj b/backend/src/app/util/shell.clj index 7ee3e5a697..0d7349d632 100644 --- a/backend/src/app/util/shell.clj +++ b/backend/src/app/util/shell.clj @@ -21,6 +21,19 @@ (set! *warn-on-reflection* true) +(defn- prlimit-cmd + "Build a prlimit command prefix from a resource limits map. + Returns nil if limits is nil/empty." + [limits] + (when (seq limits) + (let [prefix (cond-> ["prlimit"] + (:mem limits) + (conj (str "--as=" (* (long (:mem limits)) 1024 1024))) + + (:cpu limits) + (conj (str "--cpu=" (long (:cpu limits)))))] + (conj prefix "--")))) + (defn- read-as-bytes [in] (with-open [^InputStream input (io/input-stream in)] @@ -45,7 +58,7 @@ penv) (defn exec! - [system & {:keys [cmd in out-enc in-enc env timeout] + [system & {:keys [cmd in out-enc in-enc env prlimit timeout] :or {out-enc "UTF-8" in-enc "UTF-8"}}] (assert (vector? cmd) "a command parameter should be a vector") @@ -53,7 +66,10 @@ (let [executor (::wrk/executor system) _ (assert (some? executor) "executor is required, check ::wrk/executor") - builder (ProcessBuilder. ^List cmd) + full-cmd (cond->> cmd + (seq prlimit) + (into (prlimit-cmd prlimit))) + builder (ProcessBuilder. ^List full-cmd) env-map (.environment ^ProcessBuilder builder) _ (reduce-kv set-env env-map env) process (.start builder)] diff --git a/backend/test/backend_tests/shell_test.clj b/backend/test/backend_tests/shell_test.clj index 343d3af345..c77c585568 100644 --- a/backend/test/backend_tests/shell_test.clj +++ b/backend/test/backend_tests/shell_test.clj @@ -76,3 +76,32 @@ :cmd ["sleep" "0.1"] :timeout nil)] (t/is (= 0 (:exit result)))))) + +(t/deftest exec-prlimit-normal + (t/testing "normal process completes within prlimit" + (let [result (shell/exec! {} + :cmd ["echo" "hello"] + :prlimit {:mem 256 :cpu 10} + :timeout 10)] + (t/is (= 0 (:exit result))) + (t/is (str/includes? (:out result) "hello"))))) + +(t/deftest exec-prlimit-cpu + (t/testing "process exceeding CPU limit is killed" + (let [result (shell/exec! {} + :cmd ["bash" "-c" "while true; do :; done"] + :prlimit {:cpu 2} + :timeout 10)] + (t/is (not= 0 (:exit result)))))) + +(t/deftest exec-prlimit-memory + (t/testing "process exceeding memory limit is killed" + ;; Use python3 to allocate more memory than the limit allows. + ;; This test requires python3 to be available in the environment. + (let [result (shell/exec! {} + :cmd ["python3" "-c" + "import sys; x = bytearray(600 * 1024 * 1024); sys.exit(0)"] + :prlimit {:mem 256} + :timeout 10)] + ;; Should fail because 600 MiB > 256 MiB limit + (t/is (not= 0 (:exit result))))))