Uploads live outside the web root, so PHP authorizes every download and
then hands the file to the web server with a header naming it. Four
routes decided that for themselves and all four hard-coded nginx's
spelling. On Apache or LiteSpeed nothing acts on the header, so the
empty body PHP sent goes to the visitor: files upload fine, thumbnails
are broken images, and downloads arrive as 0 bytes, with every other
page working. Reported as #1765 from an Apache 2.4 install, and before
that as #1266, #1215, #870 and #1271.
It is also a regression from v1, which had a download_method setting --
php, apache_xsendfile, litespeed, nginx_xaccel -- defaulting to php. v1
therefore worked on any server out of the box and v2 did not, and a v1
Apache user migrating lost every download with nothing to tell them why.
So the four sites now go through one FileDelivery, and it picks:
auto (default) nginx when SERVER_SOFTWARE says nginx, else php
nginx X-Accel-Redirect, a URL path via the internal location
xsendfile X-Sendfile, an absolute path (Apache mod_xsendfile,
LiteSpeed)
php BinaryFileResponse
Defaulting to auto rather than nginx is the point of the change: a
default that assumes nginx leaves an Apache install exactly as broken as
it is today until somebody reads INSTALL.md. Slow beats empty.
Auto never picks xsendfile, even where the module is loaded.
mod_xsendfile also needs XSendFilePath to allow the storage directory,
which cannot be seen from here, and choosing it on the strength of the
module being present would trade a silent failure an administrator can
diagnose from the dashboard for one nobody can.
BinaryFileResponse rather than a readfile loop because it answers Range
requests. nginx does that itself on the fast path, so hand-rolling it
would have broken seeking through a video on exactly the installations
this fallback exists for. Verified end to end: 206 with the right
Content-Range through the live stack.
Two guards. Every method checks the path cannot climb out of the storage
area -- nginx resolves `..` in the URL it is handed as happily as PHP
would -- and the two methods that hand over a filesystem path resolve it
and prove it lands inside the root. Callers pass paths from rows they
just authorized, so this is a backstop; it is here because the cost of
being wrong once is handing over any file the web server can read.
The dashboard's System panel names the method, with a warning icon and a
dialog when PHP is doing the sending: what is happening, what it costs
(one worker held for the whole of each download, so a few large
simultaneous ones can occupy every worker while the processor sits
idle), why it is set that way, and the three ways out. Written to be
accurate rather than reassuring -- nothing is broken, it does not scale
-- and the notice stays even when php was chosen deliberately, because
the trade-off is the same either way. /system/settings/downloads repeats
it, which is where somebody coming from v1 goes looking for the
dropdown.
An environment variable rather than a stored setting: it describes the
server this installation runs on, not a preference, and a value in the
database travels to a different server in a restore and is wrong there.
Read only in config/projectsend.php, so config:cache cannot blank it.
The suite pins itself to nginx. Left at auto it would detect no server
at all, fall back to php, and quietly retire the coverage of the
mechanism most installations actually use.
Both preference writers validated their array as ['required', 'array']
and looped updateOrCreate over it:
'widgets' => ['required', 'array'],
'widgets.*.widget_key' => ['required', 'string', Rule::in(WIDGET_KEYS)],
Rule::in answers "is this a key I know", once per element. It says
nothing about how many elements there are, and nothing about whether they
repeat -- so a request could name the same valid key any number of times
and buy a SELECT and an UPDATE for each one.
Measured on this base, sent as JSON (a form-encoded array that size is
truncated by max_input_vars long before it reaches the controller):
widgets 10 entries 37 queries 1 row
500 entries 1044 queries 1 row
3000 entries 7051 queries 1 row
notifications 10 entries 23 queries 1 row
2000 entries 2025 queries 1 row
One row, every time. The work is not even data growth -- 3000 entries
write the same single row 3000 times, because updateOrCreate matches on
(user_id, widget_key) and every element after the first is an update of
what the one before it just wrote.
Neither route is behind a throttle: bootstrap/app.php applies
throttleApi() to the API group only, /dashboard/widgets is behind `auth`
alone and /settings/notifications is deliberately outside the `staff`
group, since every account manages its own. So the weakest account on the
installation -- a client with no permission at all -- can reach both, and
the only ceiling is post_max_size.
Both are bounded by the list they already validate against, not by a
number:
- widgets by count(self::WIDGET_KEYS), the same constant Rule::in reads.
- preferences by count($this->emailableKeys()), because
NotificationTypeRegistry is deliberately open -- "never a closed enum,
since core must not need to know a package's notification type keys at
compile time" -- so a literal would be wrong the day a module
registers one.
`distinct` on the key does the other half: a layout has at most one entry
per widget, which is what the screen sends and what the loop assumes.
After: 3000 entries cost 30 queries and write nothing, refused with a 422
instead of half-applied.
Two findings, one cause, one change -- they are the same three words in
two modules, and splitting them would leave the rule stated once and
broken once. Tests live with each controller: two refusals each, both
failing against the unfixed controllers, plus one for the largest
legitimate submission -- a full nine-widget layout, and every emailable
type at once -- so the bound can never be tighter than the screen.
resolveTransferRange() builds every boundary in the viewer's zone, deliberately: "last week" should end when their evening does, not at whatever hour UTC midnight falls on for them. Its docblock then claimed the instants "compare against the UTC column directly". They did not -- the query builder formats a Carbon in whatever zone the object carries and discards the offset, so the viewer's midnight reached the database as a UTC string. For Asia/Tokyo the window really began at 2026-08-21T15:00:00Z while the query asked for 2026-08-22 00:00:00: nine hours at each end, both in the same direction, so the first nine hours of the viewer's window were missing from the chart and the last nine hours of somebody else's day were counted into it.
The comparison now converts to UTC, one ->copy()->utc() per boundary. The copy matters: the originals keep the viewer's zone, so the day cursor and the grouping below still put an evening upload on the right bar, which is the half that really is about the viewer's calendar. Every other date filter already goes through LocalDay::start()/end(), which return UTC, which is why the activity log and the download history never had this.
Verified before merging: 20 passed on the trial-merge, 1 failed / 19 passed with app/ reset. Shares DashboardController and its test file with #1722, already merged, so the merged tree was checked -- that PR's visibleToClient change is intact.
Reported and fixed by @denkfabrik-li.
resolveTransferRange() builds every boundary in the viewer's zone, which
is right and deliberate: "last week" should end when their evening does.
Its docblock then claims the instants "compare against the UTC column
directly". They do not. The query builder formats a Carbon in whatever
zone the object carries and drops the offset, so the viewer's midnight
arrives at the database as a UTC string.
For Asia/Tokyo, measured:
the instant the window really starts 2026-08-21T15:00:00+00:00
what the query asked for 2026-08-22 00:00:00
Nine hours at each end, in the same direction: the first nine hours of
the viewer's window are missing from the chart, and the last nine hours
of somebody else's day are counted into it. Every zone east or west of
UTC gets a chart that is quietly wrong at both edges, which is worse than
one that is obviously wrong.
The comparison now converts; the day cursor a few lines below does not,
because that half genuinely is about the viewer's calendar and is what
puts an evening upload on the right bar.
One test, in Asia/Tokyo, with an upload in the first hour of the viewer's
window. Without the fix it is missing from the chart.
clientDashboard() restates the assignment half of
File::scopeVisibleToClient in a whereHas of its own. The scope is the
single source of truth for client file access and ends in notExpired(),
which the copy leaves off, so the two disagree in both directions.
Over: an expired file stays counted and keeps its name on the dashboard
after /my-files has stopped listing it and the download answers 403. Under:
everything that reaches a client another way is missing -- a file inside a
folder shared with them, a file they uploaded through the portal
themselves, and a revision, which owns no assignment row at all and
inherits its original's recipients through SharingIdentity.
Replaced by the scope itself, which is what /my-files runs. The existing
test for the page is unchanged and still passes: a directly assigned,
unexpired file counts exactly as before.
Two tests, one for each direction. Without the fix both go red.
Two things found by checking #1696 and #1699 -- open branches carrying
the same fixes I wrote this morning -- against what I actually shipped.
**topClientsByStorage was scoped with the wrong question.** 4b8220a
narrowed it with StaffLibraryScope::files(), which is right for the two
widgets that name files and wrong for the one that names clients: a
stranger client's upload can sit legitimately inside a scoped viewer's
library, shared with a group one of their own clients belongs to. So the
file was theirs to read and the uploader's name was not theirs to see.
Measured: "Stranger Client Ltd", on nobody's roster, ranked on a scoped
dashboard. assignableClientIds is what the widget is actually asking, and
it is what #1699 used. Their version was right and mine was not.
**The client guard is one method now, not eight copies.** #1696 wrote it
as a private guardTarget() rather than repeating viewer-resolve plus
abort at each site, which is better, and this is a change whose whole
argument is that a rule stated in many places drifts. Behaviour is
identical; the eight sites now read as one rule.
The published document reorders a 404 below a 422 on one path. Scramble
reads abort_unless out of a method body but not out of a helper it calls,
so the 404 now comes from route model binding instead of from the inline
abort -- same response, different position. #1701's body names this trap;
worth knowing it costs ordering and not content.
Credit where it is due: both come from denkfabrik-li's #1696 and #1699,
which were open while I was writing the same fixes. Those two are closed
against this and against e7b5b6a, 4b8220a and 67e9204.
Closing the one thing 4b8220a left open, and the reason it was left: the
expired-files widget reads StaffLibraryScope::files(), and
File::scopeVisibleToClient ends in notExpired(), so a client-scoped
viewer sees only their own expired uploads and never a client's.
Widening that would mean a library query that keeps expired rows, and
scopeVisibleToClient is the single source of truth for client file
access -- the highest-stakes function to go changing for a dashboard
widget. So the boundary stays where it is and the widget stops
overstating itself.
That matters more here than on the two widgets beside it. "Largest
files" showing the largest files somebody can see is still true from
where they stand; a warning about what is due to be deleted, quietly
narrower than it looks, reads as "nothing to worry about" on behalf of
files it never looked at. So this one gets a `scoped` flag from the
server, a title of "Your expired files", a line saying clients' files
are not listed, and an empty state that says none of *your* uploads have
expired rather than that nothing has.
Retitled at the call site rather than in WIDGET_LABELS, because the same
widget means two different things to two viewers and only the server
knows which one is looking.
Checked in a browser for both, not just in the assertions: the scoped
dashboard renders "Your expired files / Files you uploaded. Your
clients' files are not listed here. / None of your uploads have
expired.", with no console errors, and an unscoped administrator's is
unchanged.
The sweep after #1685 turned up the same leak two widgets further down
the same controller. largestFiles() and expiredFiles() already take the
viewer -- to decide whether their rows get links -- but queried with a
bare File::query(), so a client-scoped staff member's dashboard named
files belonging to clients they hold nothing of.
The note above largestFiles() says a link that 403s is accepted rather
than adding per-row scope checks. That reasoning is about the link. A row
that should not be there at all is a different problem, and the name is
the part that leaks: "Q3 delinquent accounts" says plenty without ever
being downloadable. Scoping the query is also cheaper than the per-row
check that note declined -- StaffLibraryScope builds a scoped user's
query once per request.
Reachable in the default configuration, unlike the last few of these: the
Client Manager role ships client-scoped and holds view_statistics.
topClientsByStorage() goes with them; it names clients rather than files,
which is the thing MembershipRequest::approvableBy and ActivityLogScope
already exist to keep inside a roster.
counters() and transferSeries() stay installation-wide, and now say so.
A total carries no names -- "417 files" tells a scoped viewer nothing
about whose they are -- and if that ever stops being the line, both move
together.
One consequence worth stating rather than discovering: scopeVisibleToClient
ends in notExpired(), so a scoped viewer's expired-files widget now lists
only their own expired uploads, not a client's. Safe, and under-inclusive
-- telling them about a file auto-delete is about to take needs a library
query that keeps expired rows, which is a boundary to decide rather than
to invent inside a leak fix.
#1685 fixed the dashboard rebuilding a log row by hand and dropping
`origin` from it. One layer down, the same method was skipping something
larger: it ran a bare ActivityLog::query(), so ActivityLogScope never
applied.
That scope exists for this exact case, and says so in its own docblock —
`view_actions_log` is not the whole answer for a client-scoped staff
member, because a log entry carries the subject's *name*. An unscoped log
reads out the name of every file in the installation, and who touched it,
to somebody who gets a 403 on the files themselves.
Measured before the fix, one client-scoped viewer with the permission:
/activity → []
/dashboard → Uploaded the file "Q3 delinquent accounts"
Same person, same permission, opposite answers. The activity page and the
download history both apply the scope; the dashboard was the one caller
that did not, which is the same shape of gap #1685 was about.
More reachable than it looks: the Client Manager system role ships with
`view_actions_log`, so this is the default configuration rather than
something an administrator has to build.
Two tests: a scoped viewer sees only the entry about a file in their
library, and an unscoped one still sees everything.
transferSeries() is left alone on purpose. It is unscoped too, but it
returns per-day counts with no names or subjects attached, which is a
different exposure and arguably not one at all.
Every list in /api/v1 answers "what is there now". Nothing answered
"what happened", and for the two events people most want to act on there
was nowhere to look at all.
Sharing a file writes an assignment row and never touches the file, so no
amount of polling /files?updated_since= will ever show a share. A
download is recorded only in the activity log. So the most requested
automations for a file-sharing product — tell me when a client gets a
file, tell me when they open it — were not possible to build.
GET /api/v1/activity is one feed rather than one endpoint per event,
because the log already records every one of them and a caller filtering
by action gets whatever the application grows later without waiting for
us to expose it.
It reuses what already exists: view_actions_log is the permission the
activity screen uses, and ActivityLogScope narrows the rows the same way,
so a staff member limited to their assigned clients cannot read the whole
installation's log through a token when the screen would not show it.
Two deliberate limits. Class names never reach the wire — subject.type is
a stable public string, or moving a model between namespaces would be a
breaking change to a frozen contract. And no ip_address, though the
column exists and the screen shows it: a person looking at a log has
decided to look, where an integration streams every row to somebody
else's servers by default.
PollingQuery grew an optional column so it can walk a table that is
appended to rather than edited. The parameter stays updated_since
everywhere, because on an append-only log the two timestamps are the same
thing and one shape learned once is worth more than a second name.
The Recent activity widget rebuilt each log entry inline instead of going
through ActivityPresenter, and the inline copy dropped `origin`. On the
frontend "System" and "Anonymous" are both actor_name null and only
`origin` tells them apart, so every actorless entry -- public and
share-link downloads, anonymous comments -- rendered as "System ...".
Present the entries through the shared ActivityPresenter, the same
sentence-ready shape the activity page and detail panels already use, so
the dashboard cannot drift from them again.
An actor with no personal access token has always meant a browser, and
for as long as a session and a Sanctum token were the only two ways to
authenticate, that was true. It stops being true the moment anything else
can, and the failure is silent: the action gets recorded as a person
clicking, in the one table whose whole purpose is answering "did I do
that, or did something acting for me?"
Nothing misreports today — every call site that passes an explicit actor
is a browser request, an API request whose actor carries the token, or a
console command with no actor at all. This closes the trap before the AI
connector in cloud-modules walks into it.
ActivityOrigin is a closed enum, so core has to publish both the case and
the hook before a package can use either. ResolvingActivityOrigin is
asked only in the ambiguous case: a request carrying a token is the API
and a request with nobody signed in is public or system, and neither is
in any doubt, so neither is offered — one package must not be able to
quietly relabel how every integration's actions are attributed.
The person stays the actor. They authorised it, and a log naming the
assistant instead would lose the only fact that matters. What the
connector was called goes in api_token_name, beside a null token id,
because that column means a row in personal_access_tokens and this is not
one.
The new origin is kept out of the activity filter unless the edition can
actually produce it. A filter option that can only ever return nothing is
a feature dangled at an edition that does not have it, which is the one
thing the edition boundary exists not to do.
v1 could preview four kinds of file in a modal — images, video, audio and
PDF. v2 previewed only images, and not by decision: preview shipped as part
of the image *thumbnail* work (1c68aa1), so "previewable" quietly became a
synonym for "GD can decode it". FileThumbnailController::preview() gated on
ThumbnailGenerator::SUPPORTED_MIME_TYPES, the frontend mirrored the same
four types, and the dialog was a hardcoded <img>.
Rather than widen that list — it drives pathFor(), extensionFor(),
generate() and FileDiskCleanup, and a video reaching getimagesize() is a
500 — this separates the two questions. PreviewKind now answers "may these
bytes be served inline, and what element renders them?", while
ThumbnailGenerator keeps answering the narrower "can this app decode it
itself?", which is what renditions, the cache and the watermark hook
actually depend on. Image delegates to it so the two cannot drift.
The allowlist stays a security boundary: mime_type is sniffed from the
bytes, so text/html and image/svg+xml remain excluded, and PreviewKind is
deliberately narrower than "formats a browser might cope with" — no
quicktime, avi or matroska, because an embedded player for those shows a
black rectangle. Those still download exactly as before.
docs/security-audit-2026-08-05.md finding 1 recorded that adding
application/pdf "should be a conscious decision". This is that decision,
and three things were measured rather than assumed:
- An <iframe sandbox> cannot be used. Chrome refuses to run its PDF viewer
in a sandboxed frame at all (ERR_BLOCKED_BY_CLIENT, with or without
allow-same-origin) — the attribute removes the feature, it does not
harden it.
- nginx's `Content-Security-Policy: sandbox; default-src 'none'` on
/protected-files/ does work (a <video> frame lands in an opaque origin),
but Chrome exempts its PDF viewer from it, so it is not what protects
the PDF case.
- What does is the allowlist plus the browser's own PDF sandbox, where PDF
JavaScript has no DOM and no cookies.
Range requests were verified end to end: 206 with a correct Content-Range,
a byte-perfect file reassembled from three ranges, and a real browser
seeking to 10s of a 20s clip. nginx drops the upstream Content-Length on
the X-Accel path, so there is no collision.
Two settings, both defaulting on so no installation loses what it has:
clients_can_preview_files and public_listing_preview_enabled. Staff are
never gated. The anonymous side needed a route of its own — there was no
public preview endpoint — with its own throttle bucket, since a bare
throttle: shares one counter across that whole block.
A preview now logs at most one FilePreviewed per viewer per file per five
minutes: a <video> turns one deliberate act into a long tail of Range
requests, and a row each would bury the log.
Also fixes a layout bug the tests could never catch. A portal file row was
flex justify-between with three children — name, comment trigger, download
— so the middle one settled wherever the name happened to end and the
comment icon sat at a different place on every row. The name block now
takes the slack and every action lives in one trailing group, with the
comment trigger in a fixed-width slot so the icons form a column. And
because half the previewable files have no thumbnail to click — a PDF, an
mp3 and an mp4 all render as a generic icon — every row gains an explicit
PreviewAction beside DownloadAction, matching whatever style that theme
gives its download control.
The installation-wide download history listed every download newest
first and offered nothing else, so "did that client ever actually
download the contract?" meant paging through everything that had
happened since.
It now filters by file name, by who downloaded it, and by date range,
in the same toolbar every other list uses: the query string carries the
filters, so a narrowed view is a link somebody can be sent.
Both names are matched against what the entry snapshotted rather than
through a join, so a file or an account deleted since is still findable
by the name it went out under — often exactly what this page is being
asked. The filters narrow the viewer's already-scoped query rather than
replacing it, so a client-scoped staffer cannot search their way to a
download of a file outside their library.
The activity log is where an administrator goes to answer "what changed
on this installation, and when" — and the largest change of all was not
in it. A new version arrived, the schema moved, behaviour changed, and
the log said nothing.
`projectsend:update` now records it as a system action, naming both
versions: "Updated ProjectSend to 2.1.0, from 2.0.1". It appears in the
log's own action filter without further work, since that list is built
from the enum.
Only a real version change is written. The container entrypoint runs this
command on every boot, so logging unconditionally would bury the log
under an entry per restart, and a first boot is an installation rather
than an update — SetupCompleted already covers that. "First boot" is
decided by whether any migration had run before this one, not by whether
a version was recorded: the first update of any installation older than
this command finds no recorded version, and that update is exactly the
one worth logging. It says "from an unrecorded version", once, ever.
Writing the entry cannot fail the update: an update that worked must not
report failure because its own paperwork did.
Verified on a real manual install — the row renders as
"Updated ProjectSend to 2.0.3, from 2.0.2", attributed to the system.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Client file sharing, rebuilt from the ground up: a private area per
client, resumable uploads, folders, groups and categories, sharing with
expiry dates and download limits, comments, file versions, an activity
log, a REST API, and sixteen languages.
This repository begins here. ProjectSend 2 was developed privately, and
that development history is not published — the previous generation
remains available, with its own history, at projectsend/legacy.
Free software under the GNU General Public License v2, or (at your
option) any later version.
ignacionelson