penpot/backend/test/backend_tests/shell_test.clj
Andrey Antukh b4532486e3
Add configurable resource usage limits for imagemagick (#10240)
* 🐳 Add ImageMagick policy.xml resource limits to backend Docker image

Add a restrictive policy.xml to the backend Docker image that caps
ImageMagick resource usage: 256MiB memory, 512MiB map, 128MP area,
30s time limit, 16KP max dimensions. Blocks PS/EPS/PDF/XPS coders
to prevent Ghostscript attack surface.

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>

*  Add timeout support to shell/exec!

Add optional :timeout parameter (in seconds) that uses
Process.waitFor(long, TimeUnit). On timeout, the process is
destroyed forcibly and an :internal/:process-timeout exception
is raised. Stdout/stderr readers handle IOException from closed
streams when the process is killed.

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>

* ♻️ Rename ::wrk/netty-executor to ::wrk/executor with cached pool

Replace DefaultEventExecutorGroup (fixed Netty thread pool) with a
cached thread pool (px/cached-executor) for general async task
offloading. The cached pool creates threads on demand and reuses
idle ones, which is more appropriate for blocking I/O workloads
(shell commands, message bus, rate limiting, etc.).

Changes:
- Rename ::wrk/netty-executor to ::wrk/executor in worker/executor.clj
- Switch implementation from DefaultEventExecutorGroup to px/cached-executor
- Update all ig/ref wiring in main.clj (msgbus, tmp cleaner, climit, rlimit, rpc)
- Remove ::wrk/netty-executor from redis.clj (let lettuce create its own
  eventExecutorGroup instead of sharing a Netty executor)
- Assert executor is present in shell/exec! to prevent silent nil usage
- Remove executor-threads config (no longer needed for cached pool)

The ::wrk/netty-io-executor (NioEventLoopGroup) remains unchanged as it
handles actual non-blocking network I/O for Redis and S3.

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>

* 🔥 Remove im4java dependency and replace with direct ImageMagick CLI calls

- Replace im4java Java library with direct 'magick' CLI calls via shell/exec!
- Add PENPOT_IMAGEMAGICK_* config env vars for resource limits (thread, memory, map, area, disk, time, width, height)
- Use configurable ImageMagick environment with sensible defaults matching policy.xml
- Remove -Dim4java.useV7=true JVM flag from startup scripts
- Remove org.im4java/im4java from deps.edn
- All ImageMagick commands now use shell/exec! with 60s timeout and resource limits

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>

* 💄 Rename imagemagick env functions and optimize config reads

- Rename imagemagick-defaults -> imagemagick-default-env
- Rename imagemagick-env -> get-imagemagick-env
- Optimize to avoid double cf/get calls per config key

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>

*  Add tests for shell/exec! timeout and media processing

- Add shell_test.clj: tests for exec! timeout, env vars, stdin, stderr
- Add media_test.clj: tests for info, generic-thumbnail, profile-thumbnail
- Fix generic-process to prefer explicit format over input mtype
- Fix shell/exec! to use cached executor when system has no executor
- Fix reduce-kv accumulator in set-env (must return penv)

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>

* ♻️ Refactor media/process to take system as first argument

- Change (defmulti process :cmd) -> (defmulti process (fn [_system params] (:cmd params)))
- Change (run params) -> (run system params)
- All process methods now receive [system params]
- Update all callers: rpc/commands/media, profile, auth, fonts
- Revert shell/exec! to require system with executor (no fallback)
- Fix lint warnings and formatting

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>

* 🔥 Remove unused app.svgo namespace

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>

* 🔥 Remove Node.js from backend Docker image

- Delete unused svgo-cli.js script
- Remove Node.js installation from Dockerfile.backend
- Remove svgo-cli.js copy from backend build script

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>

* 🔥 Remove unused process-error multimethod

- Remove process-error multimethod and its default handler
- Simplify media/run to directly call process
- Fix alignment in main.clj

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>

* 📚 Add ImageMagick resource limits configuration to technical guide

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>

---------

Co-authored-by: mimo-v2.5-pro <mimo-v2.5-pro@penpot.app>
2026-06-18 17:52:01 +02:00

79 lines
3.0 KiB
Clojure

;; 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.shell-test
(:require
[app.common.exceptions :as ex]
[app.util.shell :as shell]
[clojure.string :as str]
[clojure.test :as t]))
(t/deftest exec-normal-completes
(t/testing "normal process completes within timeout"
(let [result (shell/exec! {}
:cmd ["echo" "hello"]
:timeout 10)]
(t/is (= 0 (:exit result)))
(t/is (str/includes? (:out result) "hello")))))
(t/deftest exec-captures-stderr
(t/testing "stderr is captured separately"
(let [result (shell/exec! {}
:cmd ["bash" "-c" "echo out; echo err >&2"]
:timeout 10)]
(t/is (= 0 (:exit result)))
(t/is (str/includes? (:out result) "out"))
(t/is (str/includes? (:err result) "err")))))
(t/deftest exec-non-zero-exit
(t/testing "non-zero exit code is captured"
(let [result (shell/exec! {}
:cmd ["bash" "-c" "exit 42"]
:timeout 10)]
(t/is (= 42 (:exit result))))))
(t/deftest exec-with-env
(t/testing "environment variables are passed to the process"
(let [result (shell/exec! {}
:cmd ["bash" "-c" "echo $MY_VAR"]
:env {"MY_VAR" "test-value"}
:timeout 10)]
(t/is (= 0 (:exit result)))
(t/is (str/includes? (:out result) "test-value")))))
(t/deftest exec-with-input
(t/testing "stdin input is passed to the process"
(let [result (shell/exec! {}
:cmd ["cat"]
:in "hello from stdin"
:timeout 10)]
(t/is (= 0 (:exit result)))
(t/is (str/includes? (:out result) "hello from stdin")))))
(t/deftest exec-timeout-kills-process
(t/testing "process that exceeds timeout is killed and raises exception"
(let [start (System/currentTimeMillis)]
(try
(shell/exec! {}
:cmd ["sleep" "60"]
:timeout 1)
(t/is false "should have thrown")
(catch Exception e
(let [elapsed (- (System/currentTimeMillis) start)
data (ex-data e)]
;; Should complete quickly due to timeout, not wait 60s
(t/is (< elapsed 10000) "process should be killed within ~1 second")
(t/is (= :internal (:type data)))
(t/is (= :process-timeout (:code data)))
(t/is (= 1 (:timeout data)))))))))
(t/deftest exec-no-timeout-waits
(t/testing "without timeout, process runs to completion"
(let [result (shell/exec! {}
:cmd ["sleep" "0.1"]
:timeout nil)]
(t/is (= 0 (:exit result))))))