Comment out the media-storage-bytes-per-team check in
upload-file-media-object. The SUM over storage_object takes
3-12s per upload on prod (3.5TB DB, ~34 slow runs/hour)
and blocks uploads. Keep the SQL and quote definition
for re-enable with a cheaper implementation.
Update the upload quota test to document the disabled
state until the new implementation lands.
AI-assisted-by: muse-spark-1.3-contributor
Binfile v1/v3 import and the v3 export get-file step hold a single
transaction while the ZIP is decoded and the file data is realized.
Disabling idle_in_transaction_session_timeout left a stuck connection
holding locks (and a pool slot) forever.
Raise it to 30 minutes instead. The longest import/export observed in
production is under 8 minutes, so the limit still bounds a stuck
session. statement_timeout stays at 300 s.
Add set-transaction-idle-timeout and configure-import-transaction in
app.binfile.common, use them from both paths, and cover the helper
with a test.
AI-assisted-by: deepseek-v4.1-flash
* 🐛 Keep file_change revisions on their own deletion schedule
Restoring a file from the trash cleared the deletion mark on every
file_change row of that file, revisions included. A revision is born
already scheduled for deletion one hour later, and objects-gc only
looks at rows that carry a date, so unmarking one left it in the
table for ever: production had 358,517 of them across 3,614 files.
The file deletion cascade had the mirror problem: it rewrote the date
of rows that already had one, pushing a revision out to the trash
lifetime.
Restrict the restore to durable history and date only the rows that
have no date yet, so a revision keeps the schedule it was born with.
A pinning test covers the snapshot update path, which cannot reach a
revision because it resolves its row through a read filter. The
backend memories now carry the model of the three file_data classes
and this invariant, in place of the two superseded stubs.
Closes#12418
AI-assisted-by: deepseek-v4.1-flash
* 📚 Add autovacuum and Postgres exporter notes to Grafana memory
Record what this Grafana instance can and cannot show about autovacuum
so the next session does not have to re-derive it:
- The Postgres exporter scrapes the primary as well as the
read-only replica, so the primary's pg_stat_activity and
pg_settings_* are in Prometheus even though the SQL
datasource cannot reach them.
- It publishes no pg_stat_progress_* collector, and
pg_stat_activity_count has no backend_type label: autovacuum
workers are the usename/application_name-less bucket with
state=active. The launcher shows up as the AutoVacuumMain
wait event, and it is absent on the replica.
- A metric the exporter does not publish needs a custom query
block in its YAML queries file and a restart of the exporter.
- The Postgres server ships its own log to Loki (the job is
postgresql, primary only), whose streams carry only filename,
instance, job and level. The completed vacuum lines live
there: a per-table history, not a view of what runs now.
- The PostgreSQL section now ends with the vacuum panels,
with the Vacuum TXID age table last; an in-flight per-table
view would need a pg_stat_progress_vacuum collector in the
exporter itself.
AI-assisted-by: deepseek-v4.1-flash
* ✨ Extend backend histogram buckets beyond 7.5s
Give every backend timing histogram a tail past the shared 7500ms cap
so slow RPC calls, CLIMIT waits and background tasks report real
quantiles instead of clipping at the top bucket.
Request and wait metrics grow to 60s, background tasks to 300s and
Redis eval to 10s; the sub-7.5s buckets stay untouched to keep the
history comparable.
Closes#12408
AI-assisted-by: muse-spark-1.3-contributor
* 🐛 Fix flaky assertion in climit republish test
Read the last published waiting value instead of the exact
publication list. The sampler thread started with the component
runs its first tick asynchronously, and when it lands inside the
recording window it publishes the same count twice, which made
the full-suite CI run fail while focused runs stayed green.
AI-assisted-by: muse-spark-1.3-contributor
* ✨ Add file_data blob to JSON decode helper
Allow decoding file_data blobs (app.util.blob) to JSON
for investigations with jq, offline from a blob file or
from nREPL with the backend running.
AI-assisted-by: muse-spark-1.3-contributor
* ✨ Add validation step timings to update-file logs and metrics
Measure the six phases inside process-changes-and-validate
(resolve-libs, process-changes, soft and hard affected and
schema validations) with tpoint and try/finally, so timings
are recorded even when a hard validation throws.
Publish each phase to a new histogram
penpot_rpc_update_file_validation_timing with a fixed step
label using the request buckets (up to 60000 ms), and extend
the update-file debug log with the same *-ms fields plus
file, project and team ids. The deferred log only runs on
success, so failures get an inline error log with partial
timings.
AI-assisted-by: muse-spark-1.3-contributor
* 💄 Translate decode-file-data comments to English
Translate the Spanish docstrings, section headers and test
messages of the file_data decode helper to English, keeping
code untouched. The repo convention is English in code and
comments; the helper was first written in Spanish.
AI-assisted-by: muse-spark-1.3-contributor
* 🐛 Count saved versions in the version quota
The snapshots-per-file and snapshots-per-team quotes counted the
saved versions by file_change.data, a column that only the legacy-db
file data backend writes: versions keep their payload in file_data,
so the count returned 0 and a configured quota never applied.
Count by label instead, which tells a version apart with every file
data backend, and add a partial index on file_change with the same
predicate so both counts are answered by an index only scan instead
of walking every row of every file of the team (10.3 seconds in
production for a team with 829 versions).
Closes#12404
AI-assisted-by: deepseek-v4.1-flash
* ♻️ Drop the ! suffix from the snapshot quota test helpers
The check helpers assert the outcome of a command instead of mutating
anything, so they do not take the ! that this codebase reserves for
functions whose contract changes state or runs a command (the
th/command! call inside them already carries it).
AI-assisted-by: deepseek-v4.1-flash
The storage-gc-deleted task used to stream all the pending rows with
db/plan inside one long read-only transaction, blocking the table
vacuum for the whole run and generating bloat on long deletions.
Now every chunk of 25 objects is one short read-write transaction:
the SELECT reads and locks the next objects after a (deleted_at, id)
keyset cursor with FOR UPDATE SKIP LOCKED, deletes the database rows
grouped by backend and removes their backend content. Concurrent runs
never take the same chunk, and the double SELECT of the old lock-ids
step is gone.
A chunk that fails aborts its transaction: nothing of the chunk is
deleted, its locks are released, and the run skips it; the cursor
still advances to the last row read, so a failed chunk is not visited
again in the same run.
AI-assisted-by: glm-5.3-flash
* ✨ Count the application logs as a metric, by logger and level
The error panels had no metric to read: they counted log lines in Loki,
because the backend published nothing about the errors of the application
(the only thing alive in Prometheus was the http listener error count,
which measures another thing). Now there is
`penpot_log_events_total{logger, level}`.
The counter comes from a new component, `app.loggers.metrics/reporter`,
which hooks a watch on the log record like the database and mattermost
reporters do, but without a thread or a channel: the increment is in
memory, so it runs in the watcher itself. Only `warn`, `error` and `fatal`
are counted, and only the records that go through `app.common.logging`:
the lines a library writes straight to log4j2 are not counted (one in a
thousand today).
The body of the watcher is wrapped in a `try/catch` that swallows the
error on purpose: the watcher fires from the `swap!` of the log record,
inside the logging path, so an exception there would reach whoever was
logging and would take the logging executor down with it. A test pins that
a failing publication does not propagate.
The suite removes the reporter (it would count the logs of the tests and
the recording of their `mtx/run!`), so a test of its own pins the wiring.
The memories carry the counter, its two limits and why the `try/catch` is
not noise; the grafana one also brings the notes of this afternoon about
the service account, the dashboard search and Cloudflare, which were
waiting for a commit.
AI-assisted-by: deepseek-v4.1-flash
* 📚 Address the review findings on the log-events metric
The review of the counter found that the reason written for the
`try/catch` was not the real one, so the memory and the comments say now
what happens if it is removed: with `::l/sync?` the exception reaches
whoever was logging, and on the async path it dies in the uncaught
handler of the logging executor -- outside the application's logging --
where the pool replaces the worker, so logging continues and that line is
lost. They also say that a publication failing every time is silent: the
counter stays flat and nothing is written about it.
The rest are small things the review asked for: the test of the `fatal`
level (it was in the counted set and nothing verified it), the HELP of the
metric naming the three levels, the require of the test without the unused
alias that clj-kondo flagged, and the note that the `level` is lowercase
here and uppercase in Loki.
The grafana memory loses what was said twice: the two pairs of bullets
about reading the API (the service account 403s and the search that lies)
are merged, the CLIMIT notes are one bullet instead of two, and the four
bullets about editing dashboards are grouped in their own section. Its
host name is gone as well, which the first line of that file forbids.
AI-assisted-by: deepseek-v4.1-flash
The queue gauge was written by whichever limiter of a name ran last, so a
per-profile pile-up never showed: a healthy profile writing 0 overwrote the
15 of the profile that was queueing (4 of 1441 samples in production), and
a limiter whose requests never come back read 0 while 72 threads stayed
parked.
A queue is an entry and an exit, so the new gauge is accumulated: one
counter per limiter name and instance, up when a call starts waiting for a
permit, down when it stops, so a call that never leaves stays visible. It
is always written with `:val`, and a daemon sampler republishes every count
every 15s so a name whose traffic died does not go stale in the scrape;
halting the component stops that thread.
Rejections had no metric at all: penpot_rpc_climit_rejected_total counts
them by name and reason (queue-full or timeout), and the log lines now
carry queue-max, permits, code and reason, so an operator can tell whether
the limiter did its job or was saturated.
The gauges that could not tell the truth are gone: queue and permits (same
last-write-wins problem, and waiting plus the wait histogram replace them)
and http_worker_busy_threads (reads 0 on an instance with 72 blocked
threads; pool_size is the signal). No other dashboard or alert rule reads
them.
AI-assisted-by: deepseek-v4.1-flash
Add a per-process availability flag (app.health) that /readyz
reports as 503 OFFLINE while offline, so the load balancer
drains the instance and in-flight requests finish untouched.
Operator surface is app.srepl.main/set-offline and
app.srepl.main/set-online; a restart comes back online.
Covered by backend-tests.health-test; HA docs updated.
AI-assisted-by: muse-spark-1.3-contributor
Restoring a file only un-marked the database rows (`file`,
`file_media_object`, `file_change`, `file_data`, `file_thumbnail`,
`file_tagged_object_thumbnail`), not the storage objects those rows
reference. An object that was already marked for deletion stayed
marked while a live row pointed at it, and those foreign keys are `NO
ACTION`: the storage GC cannot delete it and skips it forever (and
before the GC learned to skip, it cost the whole nightly run).
Restoring a team had the same gap with its font variants.
Both restores now unmark the storage objects of the rows they bring
back: the media and the thumbnail of the file media objects, the media
of the file and object thumbnails, the `storage-ref-id` of the file
data, and the four files of a font variant. Objects referenced by other
rows are left alone.
AI-assisted-by: deepseek-v4.1-flash
objects-gc deleted every kind of object inside one transaction per proc,
looping until empty, so a single row that a foreign key still references
aborted the whole run: the task died on its first file every night, after
the earlier steps had already spent around twenty minutes. The per-file
savepoint only saved the files proc and left the same shape everywhere
else.
Now every proc streams its pending keys with db/plan inside a read-only
transaction of its own and deletes them chunk by chunk, each chunk in a
transaction of its own with FOR UPDATE SKIP LOCKED. A chunk that fails is
reported with l/err and skipped, so the run reaches the end and the next
run retries the skipped rows.
AI-assisted-by: muse-spark-1.3-contributor
`storage-gc-deleted` ran its whole batch inside one transaction: the
handler opened it and `delete-in-bulk` reused that connection, so the
"separate transaction" its comment promised did not exist. A single
row that a foreign key still references aborted that transaction, the
cursor then died on its next fetch ("current transaction is aborted")
and the task ended up throwing `unhandled exception on running task`
after rolling back the whole night of work. That is what has kept
production from deleting a single object since late April.
Now `clean-deleted` walks the pending objects with the cursor in its
own read-only transaction and every chunk runs in its own one, so a
blocked chunk is logged with its ids and skipped while the rest is
committed. `task finished` is reachable again.
The private functions also drop the `!` suffix: the namespace is all
mutation, so the mark adds nothing and reads as noise.
The test builds a blocked object (a `file_media_object` pointing at it
as its thumbnail) plus a full chunk of pending objects and one more in
the next chunk, and checks that the run finishes and that the object of
the next chunk is deleted.
AI-assisted-by: deepseek-v4.1-flash
The four `process-deleted-*-cascade` helpers of `app.srepl.main`
loaded every soft-deleted row of a table into memory (`db/exec!`)
and ran `:delete-object` inline (`wrk/invoke!`), so they could not
be used against production: `file` alone has 2.1M deleted rows.
They now share a single `submit-deleted-cascade!` that reads the
rows with a server-side cursor (`db/plan`, in a read-only
transaction) and submits one task per row, so the work goes
through the workers like any other task. The row id goes as
`label` with `dedupe` on, and the helpers accept `:limit`, `:ids`
and `:dry-run?` to run bounded batches. Public names and their
zero-arity form are unchanged.
Tests cover the task params, the row's own date (not `now`), the
`limit`/`ids`/`dry-run?` options and that each helper reads its own
table.
AI-assisted-by: deepseek-v4.1-flash
An instance left the audit events pipeline dead: every push-audit-events
answered with 429 and 69 worker threads parked for good inside a limiter.
Two code paths of one profile acquired the same two limiters in opposite
order: a command declaring its own limits (upload-file-media-object) took
process-image/by-profile and then root/by-profile, while a command with no
limits of its own took the roots first and then re-entered climit from
inside its handler with the same ids (create-file-media-object-from-url).
Each path held what the other one waited for, and no limiter had a timeout
to break the wait.
Now wrap puts the root limiters after the command's own, so they end up
outermost and every path meets them first: the nested chains carry only the
command's own limits and run inside them, which puts both paths in the same
order. impl-invoke also keeps, per thread, the set of limiters that thread
is inside of and runs the handler inline when a call asks for one it
already holds, so a handler can no longer queue behind itself.
The shipped climit.edn sets a timeout on every limiter (60s for operations
that can be slow, 30s otherwise), so no wait can park a thread for ever.
The production file lives outside the repository and has to be mirrored.
Tests cover the acquisition order, the deadlock, the reentrancy and the
timeouts of the shipped config.
AI-assisted-by: deepseek-v4.1-flash
The command declared its limit as a plain keyword next to
`::climit/key-fn`, metadata that `climit/wrap` does not read: it
takes the key only from the positional `key-fn` of the vector form.
The limit was a single shared one, so when one call got stuck it
held the only permit and every audit batch of that instance was
rejected with 429 from then on.
The test builds the limiter from the command metadata and checks
that two profiles end up with different limiters.
AI-assisted-by: deepseek-v4.1-flash
* 🐛 Sanitize smuggled SVG on binfile import
Route imported SVG storage objects through the SVG sanitizer on
all three binfile paths (v3, v1, v2), closing the second bypass
of GHSA-ffhp-m958-qxvr. Integrity checks still run on the raw
bundle bytes first; only the persisted copy is the sanitized one.
Adds a shared sanitize-imported-svg helper with unit tests and a
v3 export-tamper-import test proving smuggled scripts no longer
survive the import.
AI-assisted-by: muse-spark-1.3-contributor
* 🐛 Normalize SVG content-type on binfile import
Close the residual GHSA-ffhp-m958-qxvr bypass where a crafted
content-type spelling (uppercase, parameters) skipped the import
sanitizer. Detection now uses a shared case-insensitive predicate,
stored values are canonicalized, and the v3 re-read honors the
import size limit.
Extends the tamper tests to obfuscated spellings and adds
exhaustive v1 coverage, including the tempfile branch.
AI-assisted-by: muse-spark-1.3-contributor
* 🐛 Gate binfile content-type on known storage types
Define the complete set of storage content-types in
app.common.media and enforce it on the binfile import schema, so
unknown types fail closed instead of passing through. Detection
keeps a single normalization at the boundary with an exact
predicate, and the string helpers use cuerdas.
AI-assisted-by: muse-spark-1.3-contributor
* 🐛 Keep only produced types in storage-object-types
Drop apng, avif, penpot and plain text from the set: no flow
stores them, they only exist in the extension mapping table.
Every member must have a producing code path; unproduced types
stay out until some flow actually stores them.
AI-assisted-by: muse-spark-1.3-contributor
* 🐛 Reject unknown content-type on v1 binfile import
Enforce the storage content-type allowlist on the v1 import
path, like v3 already does via schema. Bundles declaring types
outside the set, or none at all, fail closed with the same
media-type-not-allowed error as uploads. The sanitize branch is
explicit: SVG bytes are sanitized, anything else passes through.
AI-assisted-by: muse-spark-1.3-contributor
The S3 file download opens the temp file with CREATE_NEW, so it
fails when the path already exists. tempfile creates the empty
file on reserve, which made every large download depend on the
SDK deleting the placeholder between its own retries.
Add tmp/temp-path, which reserves a unique path and registers
cleanup without creating the file, and use it for S3 downloads.
The old retry on FileAlreadyExists stays as a safety net for a
name clash.
AI-assisted-by: muse-spark-1.3-contributor
Delete the team-scoped file, project and pin relations on leave-team
and delete-team-member, require a live team membership in the file,
project, search and listing permission queries, and clean
pre-existing orphan rows with a migration. Former members now get
not-found on file actions instead of keeping read and rename access
(GHSA-v9r9-h77c-55m2).
AI-assisted-by: muse-spark-1.3-contributor
* 🐛 Sanitize SVG namespace-prefixed elements and attributes
The SVG sanitizer compared qualified tag and attribute names against
its blocklists. The XML parser keeps the namespace prefix in the
keyword name (❌script), so a script element written with a prefix
bound to the SVG namespace passed sanitization untouched and was
later served as image/svg+xml, executing in the Penpot origin.
Match tags and attributes on the local name (the part after the last
colon) instead, so dangerous elements and attributes are removed no
matter which prefix they use. Legitimately prefixed attributes such
as xlink:href keep working.
Closes#12071
AI-assisted-by: glm-5.3-flash
* ♻️ Cover prefixed SVG vectors and hoist sanitizer blocklist
Apply the code review findings on top of the sanitizer fix:
- Add a test for the javascript: href vector using a prefix bound to
the xlink namespace, the other payload variant the old exact-key
check missed.
- Add a test asserting that legitimately prefixed attributes such as
xml:space are preserved by the local-name matching.
- Hoist the dangerous-tags blocklist next to the other sanitizer
policy defs and bind the keyword name once in local-name.
Relates to #12071
AI-assisted-by: glm-5.3-flash
The event was written twice per accepted organization invitation. The
backend submitted it, and the browser then re-submitted a copy of the
props that the backend had already put in the response under
`:organization-invitation-audit` (`handle-token :team-invitation` in
`verify-token.cljs`). Both rows carried the same name with different prop
vocabularies, and the browser copy only existed when the browser finished
the flow.
Emit the event from the backend only. It now also carries the three props
that lived in the browser copy: the organization member count before the
add, the add source, and whether the invitee also joined a team. The
origin moves to the event context as `:event-origin`. The response no
longer includes `:organization-invitation-audit`, so the browser stops
emitting the event and `verify-token.cljs` drops its
`app.main.data.event` require.
The `accept-*` events of this command now share one prop vocabulary:
`:profile-id` for the accepting profile, `:invited-by` for the inviter
and `:profile-email` for the email, replacing the mix of
`:user-id`/`:user-who-send-invitation` and `:email`.
Audit consumers of `accept-organization-invitation` now see one row per
acceptance instead of two, and must read the new prop names.
AI-assisted-by: space-bunny-free
The json reader of the binfile v3 import rewrites every key of every
entry to kebab-case, nested maps included. Shape `:svg-attrs` is the one
map whose keys are camelCase react prop names, because the svg import
path runs them through `attrs->props`, so an attribute exported as
`fillRule` came back as `:fill-rule` and was stored that way.
The renderer looks the attribute up by its camelCase name, does not find
it and falls back to the default fill rule, so a shape exported with
`fillRule: evenodd` was painted without its hole, and the attributes
panel showed `fill-rule`.
`clean-shape-post-decode` already runs on every shape right after the
schema decode, for page shapes and component objects alike, so the
repair goes there: run `:svg-attrs` back through `attrs->props`, the
same transform that built the keys. It is idempotent, so shapes that
arrive correct are left untouched.
The new tests import a real export that carries the attribute, for
page shapes and component shapes.
Closes#11954
AI-assisted-by: space-bunny-free
* 🐛 Attribute audit events to the caller, not the response
prepare-rpc-event took the event profile-id from the result map
whenever it carried one, before falling back to the caller. Any
command returning a response with a :profile-id key silently
credited the action to somebody else.
get-error-report returns the report with its decoded content
merged in, and that content holds the profile that owned the
report, so privileged reads were logged against the users whose
crashes were being inspected. verify-token on a team invitation
returns the inviter's profile-id, so accepting an invitation was
logged against the inviter.
Resolution is now ::audit/profile-id metadata, then
::rpc/profile-id, then the zero uuid; the response is never
consulted. The two verify-token branches that relied on it now
declare the profile in the result metadata. Every other command
either already declared it or returns no :profile-id; all 30
registered command namespaces were checked.
The tests that pinned the old behavior are replaced by ones
covering the new contract.
AI-assisted-by: space-bunny-free
* 🐛 Coerce the audit profile-id override to a uuid
The only sanctioned way for a command to override the profile of an
audit event is the ::audit/profile-id metadata, and the value is set
by hand in a dozen commands, some of them reading it from token
claims or other sources we do not type.
schema:event requires a uuid and submit* swallows the validation
error, so a string did not fail loudly: the row was dropped silently.
Values that cannot become a uuid are now discarded with a warning
and the event falls back to the caller, which is always a valid uuid.
A uuid, the common case, exits on the first check.
AI-assisted-by: space-bunny-free
* 🐳 Add upstream diagnostics to nginx access log
Enrich every access-log line with the internal journey of the request:
the status the backend answered (us), the time spent connecting to it
(uct), the time spent waiting for its answer (urt) and the internal
address that served the request (ua).
A plain 502 line used to say nothing about where the request died. With
this format, the tail of the line classifies the failure: connection
rejected, backend accepted and hung (uct + urt under 1s), or backend
stuck until read timeout. This was the missing witness in the Sep 20
incident, where nginx received connection resets with zero timeouts and
zero rejections.
Applied both to the production image template and the devenv config.
With proxy_pass on variables there is no upstream keepalive, so uct
measures one real TCP connection per request.
Parsing the new fields (us, uct, urt, ua) on the log shipper is left to
ops, so they can be filtered in Loki.
AI-assisted-by: glm-5.3-flash
* 🐳 Add stub_status endpoint for nginx metrics
Add a dedicated localhost-only server (listen 127.0.0.1:8082) exposing
/stub_status next to every other location of the public server. Ops can
run the official nginx-prometheus-exporter as a sidecar against
http://127.0.0.1:8082/stub_status and get nginx_connections_active,
accepted vs handled, reading/writing/waiting and request rates in
Prometheus.
Binding it to localhost and its own server keeps it unreachable from
outside the host and out of the public surface, and access_log off
avoids polluting Loki with one line per Prometheus scrape. The base
image already ships stub_status compiled in, so no image rebuild is
needed.
Applied both to the production image template and the devenv config.
AI-assisted-by: glm-5.3-flash
* ✨ Expose http server gate metrics (worker and connector)
The backend already measured dispatch latency but nothing reported the
state of the "house door": the xnio worker queue and threads, and the
monitor-level listener counters. This was the exact blind spot of the
Sep 20 incident, where the server kept answering health checks while it
accepted connections and dropped them without response.
Add a periodic metrics sampler that lives and dies with the http
server (single daemon thread, 15s interval, each sample guarded so an
unexpected error does not cancel subsequent runs) and publishes:
- worker (xnio MXBean gauges): penpot_http_worker_queue_size,
busy_threads, pool_size and max_pool_size. Negative samples are
discarded: the MXBean transiently reports -1 on the busy thread
count (verified live), and a stale negative would read as zero.
- listener (Undertow connector statistics, enabled via the new
:server/statistics yetti option): penpot_http_connector_active*
_connections gauge and requests_total / errors_total counters.
Undertow exposes absolute totals, so the sampler keeps a watermark
atom and publishes deltas, skipping (and moving forward past) a
counter reset.
The connector-level part depends on yetti v11.11, which now accepts
a :server/statistics server option (patch authored and released
upstream; before it, ListenerInfo#getConnectorStatistics always
returned nil).
New tests cover the samplers with fake MXBean/collector statistics
against real prometheus collectors, including the negative-sample
filter, the delta/watermark logic and the sampler lifecycle.
AI-assisted-by: glm-5.3-flash
* 🐛 Include jdk.management in the backend runtime JRE
The production image builds a trimmed JRE with jlink and omitted
jdk.management. Without that module the OS MXBean is
sun.management.BaseOperatingSystemImpl, which has no
getProcessCpuTime, getOpenFileDescriptorCount nor
getMaxFileDescriptorCount. The prometheus client StandardExports
reads those getters reflectively and collect() swallows the
NoSuchMethodException, so process_open_fds, process_max_fds and
process_cpu_seconds_total silently disappeared from /metrics while
the other process_* families kept flowing.
Verified against Prometheus: the app job only ever exposed
process_start_time_seconds, process_virtual_memory_bytes and
process_resident_memory_bytes; the fd and cpu families were absent.
Reproduced locally by running the backend metrics registry on a JRE
built with the same jlink module list (false/false/false) and on one
with jdk.management added (true/true/true).
Add the module to --add-modules and pin the metric contract with
backend-tests.metrics-test.
AI-assisted-by: deepseek-v4.1-flash
* ♻️ Build the http metrics sampler on promesa.exec
Replace the hand-rolled ScheduledThreadPoolExecutor and ThreadFactory
with promesa.exec primitives: px/scheduled-executor with a daemon
thread factory, and a px/schedule chain that reschedules the next
sample when the current one finishes.
Beyond fitting the existing periodic-task pattern (worker/cron,
rpc/rlimit), the chained schedule makes the docstring promise real:
with scheduleAtFixedRate an exception escaping the runnable cancelled
the following executions, while the reschedule now happens in a
finally block.
The sampler shutdown uses px/shutdown-now (shutdown! is deprecated in
promesa 12.0.0) to cancel the pending sample, keeping the previous
halt semantics.
The lifecycle test moves to the promesa predicates and a new test
covers the error-resilience promise: the first sample runs, throws,
and the next one is still scheduled.
AI-assisted-by: deepseek-v4.1-flash
* ♻️ Tighten the http metrics samplers
The samplers are leaf functions: they receive what they need and
publish it. Drop the internal nil guards (if there is no metrics
instance or no mxbean there is nothing to call them for) and move the
checks to the boundary, where the optional data is resolved:
sample-http-metrics now short-circuits with some-> and when-let.
Write the four worker gauges as four static operations instead of a
vector of pairs walked by doseq: the set is fixed, so the collection
only adds an allocation and hides each operation.
Drop the ! suffix from the sample-*-metrics family: ! marks a function
whose contract is to mutate state, while these report, and the mutation
happens in the mtx/run! they call. The constant true return, which only
existed so the removed guard tests could assert it, goes away too.
Tests follow the move: the internal-guard tests are replaced by one
boundary test (a nil server publishes nothing).
AI-assisted-by: deepseek-v4.1-flash
* 📚 Add the function design rules memory
Document the rules that came out of the http metrics sampler review:
preconditions are checked at the boundary instead of re-checked in the
core, optional-by-design data is guarded where the optionality is born,
a fixed set of operations is written statically, ! marks mutation and
not reporting, and production code is not shaped for tests.
Also state in the memory maintenance guide that memories must not use
manual line wrapping.
Linked from critical-info so it is read when designing a solution or
an API, not only when touching the samplers.
AI-assisted-by: deepseek-v4.1-flash
* 📚 Unwrap the critical-info memory lines
The memory maintenance guide forbids manual line wrapping, so rewrite
critical-info with one line per bullet and paragraph. A stray `*` at
the start of one continuation line is dropped.
AI-assisted-by: deepseek-v4.1-flash
* ♻️ Drop the redundant guard in the http server halt
create-metrics-sampler always returns the scheduler, so the sampler is
always present when integrant calls halt-key!; the nil check was dead
code, same as the yt/stop! call next to it.
AI-assisted-by: deepseek-v4.1-flash
* ✨ Add srepl helper to delete profiles by email
Add `delete-profiles-by-email!` to app.srepl.main. It accepts a
single email, a comma separated list of emails or a coll of emails,
resolves each profile, logs it to audit and enqueues the
delete-object task. The deleted-at is backdated with the configured
deletion-delay so profiles and their owned teams are purged on the
next gc pass.
Extract the per-email deletion logic into a private fn and reuse it
from `delete-profiles-in-bulk!`. Add tests for the new
`parse-emails` helper.
AI-assisted-by: glm-5.3-flash
Shared-key callers (exporter, admin-console) arrive as keywords on
auth-key-id, so transit persisted them as ~:exporter while regular traffic
stored app. Coerce with d/name at the single origin so every audit and
telemetry copy carries a plain string. Adds regression tests for the origin
and the push-audit-events path, including caller spoofing precedence.
Closes#11628
AI-assisted-by: muse-spark-1.3-contributor
* ✨ Restrict optional RPC ids to user-provided UUIDs
Add ::sm/user-provided-uuid, backed by a version and variant
aware regex that only accepts v4, v7 and v8 instances. Use it
for the optional :id of the creation RPC commands so reserved
versions such as v3 are rejected at validation time. Reads
such as get-team keep the lax ::sm/uuid. Cover the predicate
and the schema on both JVM and JS runtimes.
AI-assisted-by: muse-spark-1.3-contributor
* ✨ Cover id version restriction at the RPC boundary
Add backend regression tests proving the seven creation commands
reject reserved-version ids (v3) with :params-validation and
accept v4 ids (plus v7/v8 on create-team) through the real
decode and validate path. Also drop two duplicated assertions
and document the version and variant of every fixture UUID in
user-provided-test.
AI-assisted-by: muse-spark-1.3-contributor
The management API shared-key-auth middleware was using the standard = operator for key comparison, which is vulnerable to timing attacks. The RPC middleware already uses constant-time comparison via MessageDigest/isEqual.
This change:
- Makes constant-time-eq? public in app.http.middleware
- Updates app.http.management/shared-key-auth to use mw/constant-time-eq?
- Fixes an inconsistency where the nil-key branch returned a 2-arg function
- Adds comprehensive tests for the management shared-key-auth middleware
Closes#11426
AI-assisted-by: qwen3.7-plus
* ✨ Add expires-in option to create-demo-profile
Allow passing an optional expires-in duration when creating a demo profile so its purge is scheduled sooner than the global deletion delay. Values below 5 minutes or above the global delay are rejected with an invalid-expires-in validation error, resolved before any profile is created.
Closes#11573
AI-assisted-by: muse-spark-1.3-contributor
* 🐛 Make duration schema decoding total instead of throwing
parse-duration returned by the duration schema decoder threw DateTimeParseException on invalid strings, escaping params validation as a raw error. It now returns the input unchanged so invalid values fail the duration predicate with a clean params-validation error. Closes#11573 AI-assisted-by: muse-spark-1.3-contributor
* 📎 Fix doc version for expires-in change entry
The expires-in change entry was documented under 2.20 but the current version is 2.18.
AI-assisted-by: muse-spark-1.3-contributor
Skip the limited newsletter report when the public-uri host
belongs to penpot.dev or penpot.app, so the SaaS never sends
subscriber emails to its own telemetry endpoint.
Defer the subscriptions query with delay so it only runs when
a report is actually going to be sent.
AI-assisted-by: muse-spark-1.3-contributor
* 🐛 Reject duplicate chunk index in chunked uploads
Repeat uploads of the same chunk index each stored a new
object because upload-chunk only checked index bounds. Run the
handler in a transaction, lock the session row and reject an
already-stored index with :duplicate-chunk-index.
Also harden assemble-chunks to require exactly indices 0..n-1
so gaps or duplicates fail instead of assembling a corrupt
file. Covers media, fonts and binfile through the shared
helper.
Closes#11634
AI-assisted-by: muse-spark-1.3-contributor
* ✨ Cap upload chunk size at 30 MiB by default
Chunks were only bounded by the 350 MiB HTTP body limit while the
30 MiB caps applied to the assembled file. Add :upload-max-chunk-size
(default 30 MiB, tunable via env) and reject oversize chunks in
upload-chunk with :validation/:chunk-too-large before anything is
stored. App clients slice at 25/10 MiB, so no frontend change needed.
AI-assisted-by: muse-spark-1.3-contributor
* 🐛 Fix tx-run! call and storage resolve in upload-chunk
Pass cfg as first arg to db/tx-run!, which expects [system f & params; without it every chunk upload raised invalid system/cfg provided and no chunk was stored, breaking assemble with missing-chunks. Also resolve storage without reuse-conn: put-object! writes to the backend outside any transaction, so reusing the tx connection gives no atomicity. Media, font and storage suites green, lint and format clean. AI-assisted-by: muse-spark-1.3-contributor
Fix LDAP injection vulnerability (T5-N1-03) where the client-supplied email was used directly in the LDAP search filter without escaping RFC 4515 special characters (*, (, ), \, NUL), and the profile email was taken from client input instead of the LDAP directory attribute.
Changes:
- Add escape-ldap-filter-value per RFC 4515 section 3
- Apply escaping in search-user before building LDAP filter
- Add get-attr helper for multi-valued LDAP attributes
- Fix retrieve-user to use directory email (attrs-email) instead of client email
- Use cuerdas.core instead of clojure.string
Closes#11084
AI-assisted-by: mimo-v2.5-pro
The event batch sent to the telemetry server was encoded as a
fressian+zstd base64 blob. Send it as a plain vector of event maps
instead: the JSON encoder handles UUID and temporal types natively,
the payload becomes inspectable, and the receiver schema coerces
values back to proper types.
The receiver (penpot-telemetry) now accepts both the blob and the
plain vector, so it must be deployed before this backend change.
AI-assisted-by: omen-alpha
Add optional skip-onboarding param to create-demo-profile. When true, the demo profile is created with onboarding-viewed and release-notes-viewed set, so it skips the onboarding flow. Default keeps the current behavior. Cover both cases with RPC tests. AI-assisted-by: muse-spark-1.3-contributor
* ⬆️ Update pnpm and its deps
* ⬆️ Update JVM dependencies in backend and common
Update several JVM dependencies across backend and common:
- passay 1.6.6 -> 2.0.0 (package reorg, ctor-based rules)
- siphash 2.0.0 -> 3.0.0 (SipHasher* renamed to SipHash*)
- lettuce-core, guava, sqlite-jdbc, jsoup, lz4-java, markdown-clj,
awssdk s3/sts, selmer, jackson-core/databind, shadow-cljs
Adapt passay validation to the new API (moved packages, constructor
configuration) and siphash to the renamed classes. Add tests for
password validation and UUID advisory-lock hashing.
AI-assisted-by: deepseek-v4-flash
* ⬆️ Update node on docker images
* 📎 Minor fixes related to pnpm12 compatibility
The objects-gc task was performing a hard delete on profiles
without cascading the soft-delete to owned teams, projects, and files.
This left orphaned objects that were never cleaned up.
Now the task invokes delete-object before the hard delete, ensuring
all owned resources are properly marked for deletion and cleaned up
in subsequent GC iterations.
AI-assisted-by: qwen3.7-plus
Fix two security vulnerabilities in comment RPCs when accessed
via share-links:
- GHSA-4p97-v4wg-jxfx: Share-link holders with who-comment=team
could bypass the restriction and comment. The check-comment-permissions!
function treated can-read as sufficient, but share-links always set
can-read=true.
- GHSA-fwm4-hm9f-rmcp: Comment query RPCs returned threads from all
pages, ignoring the share-link's :pages restriction.
Changes:
- files.clj: Differentiate :membership vs :share-link in
check-comment-permissions!. For share-links, require
has-comment-permissions? only (who-comment=all).
- comments.clj: Filter threads by (:pages perms) for share-link
access in get-comment-threads, get-comment-thread, and get-comments.
Closes#11370
AI-assisted-by: qwen3.7-plus
Add owner protection to ::delete-team-member RPC command.
Previously, a team admin could remove the team owner, permanently
locking them out of their team and all resources.
Changes:
- Fetch target member data before deletion
- Validate member exists (return :not-found if not)
- Reject removal if target is owner and caller is not owner
This mirrors the existing protection in update-team-member-role.
Closes#11367
AI-assisted-by: qwen3.7-plus
* 🐛 Block IPv6 transition addresses in SSRF guard
The outbound HTTP SSRF blocklist did not classify NAT64
(64:ff9b::/96), 6to4 (2002::/16) or Teredo (2001:0000::/32)
addresses, whose embedded IPv4 target is invisible to the JVM
InetAddress predicates, so URLs resolving to them could reach cloud
metadata, loopback or RFC 1918 hosts from webhook delivery and media
import.
Transition ranges are now rejected outright and any embedded IPv4 is
re-checked against the full blocklist, including operator-supplied
extra blocked CIDRs.
Closes#11319
* ♻️ Remove dead embedded-IPv4 re-check from SSRF guard
The previous commit added a recursive re-check of the IPv4 embedded in
NAT64/6to4/Teredo addresses, but the `or` in `blocked-address?`
short-circuits on the truthy keyword returned by `transition-prefix`,
so the embedded-IPv4 branch was unreachable. The transition ranges are
already rejected outright (fail-closed), making the re-check both
unnecessary and untested.
Remove `transition-embedded-ipv4`, simplify the IPv6 branch to a plain
prefix check, and correct the docstrings and tests to match what the
code actually does.
AI-assisted-by: glm-5.3-flash
* 🐛 Filter share-link tokens in get-view-only-bundle response
The get-view-only-bundle RPC command returned all share-link tokens for a file, allowing an anonymous holder of a restrictive share-link to enumerate and use more permissive tokens.
When authenticating via a share-link, the response now only includes the share-link used for authentication, preventing token disclosure and scope escalation.
Implemented using TDD:
- RED: Test demonstrates vulnerability (all tokens visible)
- GREEN: Filter share-links when (:type perms) = :share-link
- Verified all existing tests still pass
Closes#11285
AI-assisted-by: qwen3.7-plus
* 🐛 Add membership-side test for share-link token visibility
Add test coverage for the allow side of the share-link token filtering:
team members and file owners should still see all share-links, while
anonymous share-link holders only see their own token.
This protects the (:type perms) = :share-link guard from accidental
regression that could break the owner's share-link management dialog.
AI-assisted-by: qwen3.7-plus
* ⚡ Optimize demo user setup for performance tests
Use UUID-based demo emails to prevent concurrent profile collisions.\nUse fast PBKDF2 hashing for demo profiles while keeping regular user hashing unchanged.\nAdd focused coverage for hashing, email uniqueness, and the feature flag.\n\nAI-assisted-by: gpt-5.6-luna
* 🐛 Harden font upload test setup
Report upload-session errors before chunk validation.
Skip chunk uploads when the session ID is invalid.
Remove unnecessary Mockery state from the foreign-font test.
AI-assisted-by: gpt-5.6-luna
* ✨ Add demo profile purge task
Schedule delayed deletion for demo profiles through the worker system.
Restore normal profile filtering and cover the purge handler with tests.
AI-assisted-by: gpt-5.6-luna
* 🐛 Add configurable limits for ZIP entry count and object size in v3 import
Add binfile-import-max-zip-entries (default 500,000) and
binfile-import-max-object-size (default 100 MiB) config entries.
Both are configurable via PENPOT_BINFILE_IMPORT_MAX_ZIP_ENTRIES and
PENPOT_BINFILE_IMPORT_MAX_OBJECT_SIZE env vars.
Entry count is checked before processing begins. Per-object size is
checked after each storage object content is resolved.
AI-assisted-by: mimo-v2.5-pro
* 🐛 Enforce actual decompressed byte limits on v3 import
The previous object-size check trusted the ZIP entry header's declared
size (ZipEntry.getSize()), which a malicious zip-bomb can forge. The
check would pass, then the full decompressed payload would be read
anyway during hashing and storage persistence.
Add size-limiting-stream, a FilterInputStream wrapper that counts
actual bytes read and raises :validation :max-file-size-reached when
the configured limit is exceeded. Wire it into zip-entry-storage-content
so both the hash calculation and storage write paths are bounded by
real decompressed bytes, not declared header size.
Also wire import limits into management.clj (clone-template) and
debug.clj (import-handler + clone path) for defense-in-depth, and
add a test that exercises the object-size limit with a real storage
object in the exported ZIP.
AI-assisted-by: mimo-v2.5-pro
The handler previously allowed any team member (including viewers) to
generate invitation tokens. Now requires at least edition-level
permissions (can-edit, admin, or owner).
Closes#11358
AI-assisted-by: longcat-2.0
* 🐛 Enforce ownership check on tempfile bucket access
The upload-tempfile RPC stores profile-id with tempfile objects, but
objects-handler never verified the requester was the owner. Any
authenticated user who knew the UUID could access the tempfile.
Add ownership check: tempfile bucket now requires the request's
profile-id to match the stored profile-id. Returns 404 on mismatch
(not 403) to avoid leaking object existence.
Legacy tempfiles without stored profile-id remain accessible to any
authenticated user for backward compatibility.
Closes#11269
AI-assisted-by: qwen3.7-plus
* ♻️ Extract tempfile-bucket constant and fix docstring indentation
Extract the 'tempfile' bucket string literal into a named constant
(sto/tempfile-bucket) to prevent typos and make future bucket renames
trivial. Updated 9 occurrences across 7 files.
Also fixed minor docstring indentation inconsistency in authenticated?
function.
AI-assisted-by: qwen3.7-plus
* ♻️ Refactor process-bucket! and authenticated? helpers
Replace case with cond in process-bucket! to properly resolve
sto/tempfile-bucket var from another namespace (case does not
evaluate qualified vars at compile time).
Redefine authenticated? in terms of request-profile-id to remove
duplicated lookup logic.
Closes#11269
AI-assisted-by: mimo-v2.5-pro
