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
The redis cache keys for the file library summary were built as
penpot.library-summary.<file-id>, missing the tenant component used
by the rest of the shared-infrastructure keys (rlimit, msgbus,
worker queues).
Build the key with a new file-summary-cache-key function that appends
the configured tenant, following the penpot.library-summary.<tenant>.<file-id>
pattern. Stale keys in the old format expire on their own with the
existing 30-day TTL.
Closes#11407
AI-assisted-by: glm-5.3-flash
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
* 🐛 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
* ♻️ Simplify storage GC delays and add skip-delay task params
The touched GC no longer applies an extra deletion-delay when marking
storage objects as deleted. By the time a storage object is touched, its
referencing domain row has already passed its own deletion delay, and the
reference scan is the only safety check needed. Touched objects are now
marked with deleted_at = now, so the deleted GC removes them on the next
run.
For the tempfile bucket, upload chunks now set touched-at in the future
(1h, aligned with the upload-session-gc TTL) instead of relying on a
special-case deletion delay.
Task handlers now read their task props:
- storage-gc-touched accepts :skip-delay to process all touched objects
immediately, bypassing the min-age threshold.
- objects-gc accepts :chunk-size and :skip-delay to process recently
deleted rows without waiting for the deletion delay.
This allows running the deletion cascade immediately from the REPL via
run-task! with the skip-delay option.
AI-assisted-by: deepseek-v4-flash
* ✨ Add storage object status lifecycle, verified dedup, and deletion retry tracking
Storage object lifecycle hardening:
- Add status column ('valid' | 'pending') as write-ahead marker for
object creation. put-object! inserts in 'pending' state, writes blob,
then promotes to 'valid'. Failed writes remove the pending row.
- Add :storage-pending-gc task to reclaim orphaned pending rows (e.g.
after crash between blob write and promotion).
- Verify blob existence on every dedup hit via exists-object? (fs stat /
s3 headObject). Missing blobs mark the row as deleted and create fresh
object.
- Add deletion_attempts column (migration 0154) to track physical blob
deletion attempts. Restructure gc_deleted to use chunked processing
with per-chunk transactions (short lock duration). Failed deletions
are deferred to tomorrow (deleted_at = NOW() + 1 day) to prevent
infinite loops. After 7 attempts, give up and accept orphan.
- Change del-objects-in-bulk contract to return #{fail-ids} for precise
per-id tracking (fs and s3 backends updated).
- Use tmp/tempfile for fs atomic writes with cleanup queue registration
(crashed-JVM temp files swept ~60min later). Document ATOMIC_MOVE
POSIX-only assumption.
- Add linear backoff to s3 exists-object? retries (100ms/200ms/300ms).
- Wrap compensating delete in put-object! catch block to prevent
masking original error when connection is aborted.
- Fix assert messages in pending_gc.clj and gc_deleted.clj (pool
assertion said 'expected valid storage' instead of 'db pool').
- Add pending-objects-excluded-from-gc-deleted test. Use unique path in
put-object-write-failure-leaves-no-row test to avoid collisions.
AI-assisted-by: qwen3.7-plus
* 🐛 Fix review comments on gc-deleted and storage
- Fix process-chunk! returning nil causing (+ acc nil) crash
- Add FOR UPDATE SKIP LOCKED to sql:get-deleted-chunk to prevent
infinite loop when another worker holds locks
- Pass :cause to log messages in gc_deleted.clj and s3.clj
- Fix extra space in log hint string
- Remove unused ::blob-missing? reference from storage memory
- Rename test to match actual behavior (leaves pending row)
- Add test for gc-deleted giving up after max attempts
AI-assisted-by: qwen3.7-plus
* ✨ Auto-link libraries during import based on slugified name
When a Penpot file is exported without bundled libraries and then
imported into a different environment, external library links are
broken because library UUIDs differ across environments.
This feature adds a heuristic to auto-relink libraries by matching
slugified library names against shared files in the target team:
- Export: embed external library metadata (id, name, slug, used-by)
in the manifest when libraries are not included in the export.
- Import: resolve external libraries by slugifying shared file names
in the destination team and matching against manifest slugs.
- Single match: auto-link silently (creates file-library-rel row).
- Multiple matches: emit SSE event so the frontend shows a selection
dialog for the user to pick the correct library.
- No match: import continues without linking (current behavior).
Backend changes:
- Extended manifest schema with optional :external-libraries field
- Added slugify-name, get-files-names, get-shared-files-for-team,
find-shared-files-by-slug helpers in app.binfile.common
- Threaded team-id into import cfg from RPC layer
- Added resolve-external-libraries and auto-link-libraries in v3
- Emit :library-candidates SSE event for multi-match cases
Frontend changes:
- Worker captures library-candidates SSE events and forwards them
- Import dialog shows auto-link notification and multi-match
selection UI with select dropdowns
- Added link-files-to-library! RPC helper for user selections
- Added en/es translations for new UI strings
Closes#9263
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
* ✨ Add UI for the auto-link plumbing
* ♻️ Consolidate auto-link libraries with unified export-type and fix ref integrity
Consolidates the auto-link libraries feature into a single coherent implementation:
- Unify boolean flags (embed-assets, include-libraries, link-later)
into single ::bfc/export-type parameter
- Fix critical reference-integrity bug: pre-resolution no longer remaps
:component-file refs when no link is created (multi-match / no-permission)
- compute-link-decisions as single source of truth for auto-link logic
- 80+ backend tests covering round-trip, cross-team, permissions,
edge cases, and reference integrity
AI-assisted-by: longcat-2.0
---------
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
Co-authored-by: Eva Marco <evamarcod@gmail.com>
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
* 🐛 Add accumulated storage byte quota for media uploads
Add media-storage-bytes-per-team quote to prevent persistent DoS via
repeated uploads. The quota sums storage_object sizes from both
file_media_object (media + thumbnails) and team_font_variant
(otf/ttf/woff1/woff2). Default limit is 20 GiB per team, configurable
via PENPOT_QUOTES_MEDIA_STORAGE_BYTES_PER_TEAM.
The check is invoked in upload-file-media-object before processing,
looking up the team-id via file -> project -> team_id join.
AI-assisted-by: mimo-v2.5-pro
* 🐛 Fix deduplicated storage overcounting in media-storage-bytes-per-team quote
The SQL query sql:get-media-storage-bytes-per-team used UNION ALL
across six SELECT branches that each produce a so_id reference.
When deduplication causes multiple file_media_object or
team_font_variant rows to point at the same storage_object, UNION ALL
counts that objects size once per reference — inflating "used bytes"
and causing false :max-quote-reached rejections.
Change all five UNION ALL to UNION so that duplicate so_id values are
collapsed before the JOIN storage_object / SUM(so.size).
Add a test (media-storage-bytes-quote-deduped) that creates one
storage_object referenced by two file_media_object rows and asserts
the computed usage reflects the deduplicated physical size, not 2x.
AI-assisted-by: mimo-v2.5-pro
The get-page RPC command did not validate that the requested page-id was within the share-link's authorized :pages set, allowing share-link holders to read out-of-scope pages. The get-file-fragment command had the same issue.
This fix adds page scope validation to get-page, rejecting requests for pages not in the share-link's :pages set with a :not-found error. For get-file-fragment, share-link access is denied entirely as fragments lack direct page-id mapping.
The fix aligns these commands with the existing correct behavior in get-view-only-bundle, which already filters pages based on share-link scope.
Closes#11281
AI-assisted-by: qwen3.7-plus
* 🐛 Add ownership check to share-link deletion
The delete-share-link RPC command only verified file-level edit
permission but did not check if the caller owned the share-link.
This allowed any file editor to delete share-links created by
other users, disrupting collaborative workflows.
The fix adds an ownership check that allows deletion only by:
- The share-link creator (owner-id matches profile-id)
- File admins (is-admin permission)
- File owners (is-owner permission)
Implemented using TDD:
- RED: Test demonstrates IDOR vulnerability (editor can delete)
- GREEN: Ownership check prevents unauthorized deletion
- All existing tests continue to pass
Closes#11289
AI-assisted-by: qwen3.7-plus
* 🐛 Add test coverage for share-link deletion escape hatches
Address code review feedback for PR #11290:
- Add test for editor deleting their own share-link
- Add test for admin deleting editor's share-link
- Add test for owner deleting editor's share-link
- Remove redundant :is-owner check (already included in :is-admin)
- Add clarifying comment about :is-admin including :is-owner
Closes#11289
AI-assisted-by: qwen3.7-plus
Restrict version parameter to supported values (1 or 3) via schema
validation instead of accepting any integer. Add content-based format
detection when version is not provided, using bfc/parse-file-format
to inspect file magic bytes.
Closes#11105
AI-assisted-by: qwen3.7-plus
The create-upload-session RPC method accepted total-chunks values of 0
or negative numbers without validation, creating inconsistent session
state. Add {:min 1} constraint to the schema to reject invalid values
at input validation.
Closes#11103
AI-assisted-by: qwen3.7-plus
The clone-file-media-object RPC command only checked edit permissions
on the destination file. The source media object was fetched directly
by UUID without verifying the caller had access to the file that owns
it.
This fix adds a read permission check on the source file before
cloning. If the caller lacks read access to the source file, the
operation fails with :not-found to avoid leaking information about
the existence of files/media the caller cannot access.
Closes#11087
AI-assisted-by: qwen3.7-plus
Prevent BOLA in chunked upload assembly by verifying session
ownership. The assemble-chunks function now requires a profile-id
parameter and scopes the upload_session lookup accordingly, matching
the pattern already used by upload-chunk.
All three callers (assemble-file-media-object, create-font-variant,
import-binfile) updated to pass the authenticated profile-id.
AI-assisted-by: mimo-v2.5-pro
Omit nil optional profile fields before frontend schema validation and RPC persistence. Preserve omitted language and theme values in backend updates, and add regression coverage for partial profile saves.
AI-assisted-by: gpt-5.6-luna
Round bucket reset intervals up to whole milliseconds before adding them to an instant. This prevents Clojure ratios from reaching duration conversion and disabling rate limiting for the request.
Add a regression test for a refill rate that produces fractional milliseconds.
Closes#11253
AI-assisted-by: gpt-5.6-luna
Add media type validation to upload-tempfile and upload-org-logo
management endpoints. Both stored user-supplied mtype without
checking against an allowlist. Only image types and PDF are
permitted. Non-public bucket assets now also carry
Content-Disposition: attachment to prevent inline rendering.
AI-assisted-by: mimo-v2.5-pro
Add role-ceiling check to create-team-invitations and
update-team-invitation-role methods. These RPC methods allowed
team admins to grant or elevate invitations to :owner role,
bypassing the protection that exists in update-team-member-role.
The fix replicates the existing check from update-team-member-role:
reject promotion to :owner when the caller is not an owner.
Closes#11098
AI-assisted-by: qwen3.7-plus
Apply climit with 4 global permits and 1 per-profile permit (queue 2)
to prevent connection pool exhaustion from concurrent imports. Each
import holds a DB connection for its entire duration with idle
transaction timeout disabled, so unbounded concurrency could exhaust
the pool (default 60 connections).
AI-assisted-by: mimo-v2.5-pro
When a profile is deleted, only the current session was being
invalidated. Other active sessions on different devices remained
functional until the background cleanup task completed.
Add session/invalidate-all helper that deletes all sessions for
a profile by profile_id, and call it from delete-profile before
the response transform. This ensures immediate access revocation
across all devices when an account is deleted.
Closes#11114
AI-assisted-by: qwen3.7-plus
Replace the placeholder rlimit.edn with a real per-endpoint
configuration covering auth, SSRF, search, email, media and project
operations. The previous file only had a commented-out example, so
all limits fell back to the 200k/h default window.
Also propagate the evaluated `now` timestamp into both bucket and
window result maps, so consumers (e.g. soft-mode reports) can know
exactly when the limit was checked.
AI-assisted-by: minimax-m3
Share link IDs function as capability secrets — anyone possessing
the ID can read a file without authentication. The previous UUIDv8
scheme is predictable (56 bits fixed per process + 48-bit timestamp).
Changed to uuid/random (UUIDv4) for genuine unpredictability.
Closes#11116
AI-assisted-by: qwen3.7-plus
`create-font-variant` destructures `uploads` and never reads it: the
handler passes the whole `params` map to `prepare-font-data-from-uploads`.
`clj-kondo` reports it as an unused binding and exits 2, which fails the
Lint step of the Backend workflow, and the Lint step runs before the
tests, so no branch based on `develop` can run the backend suite at all.
AI-assisted-by: mixed models
Replace the inline organization map in schema:create-organization-invitation with cto/schema:organization-with-avatar, eliminating schema duplication and fixing mismatched validation rules for :logo and :sso-active fields.
AI-assisted-by: mimo-v2.5
Prevent cross-team font injection by checking that when a font-id
already has variants, they belong to the same team. This closes a
BOLA gap where a user with team edit permissions could create a
font variant referencing a font-id from another team.
AI-assisted-by: mimo-v2.5-pro
