Two seat counters and one folder-delete refusal, in all sixteen locales.
Additive only: nothing already in a catalogue was reordered or reworded,
so the diff is three lines per file.
Polish, Czech and Russian get three plural forms where the English has
two. Those languages inflect a noun by the number in front of it -- one
case for 2-4, another for 5 and up -- and the framework's selector picks
between three segments for them, so writing only the English pair would
have produced "5 pliki" where it has to be "5 plikow". Verified through
trans_choice at 1, 3 and 7.
Five more facts for whatever watches an installation from outside the
container, and one seam so a package can add its own.
Storage is the one that was about to be wrong. It is summed from the rows
that record it, not measured on the volume: measuring the directory was
correct until external storage went live and silently stopped being, since
an upload that resolves to a bucket leaves nothing on disk to measure. A
figure taken from the filesystem freezes while the account keeps filling,
and on a managed installation that figure is what a customer is shown and
billed against. `by_disk` splits the same sum by where the bytes went,
which is the only way to see what is still sitting locally from before a
cutover. Trashed files are excluded because they hold no bytes -- File's
deleted hook takes them.
Health is what a container cannot show from outside. A queue worker dying
is invisible to anything watching the process: it is still up, and zips
quietly stop building while mail stops going out. Same for a deploy whose
migrations failed -- the application answers every request and is a schema
behind. An unreachable queue reports null rather than zero, because an
unreachable Redis is not an empty queue and reading the second as the
first is how a dead worker looks healthy.
The two-factor enforcement setting is echoed back the way EnforceTwoFactor
reads it, fallback included: reporting a stricter rule than the middleware
actually applies would be worse than reporting none.
And ResolvingInstallationStatus, so a package can report what core cannot
know. The managed storage backend and the version of the package providing
it live in cloud-modules, which this repository must not reference, and a
platform that writes eight environment variables only ever knows what it
asked for. Those came apart once: a bucket provisioned, a token minted,
every variable correct, and an image whose copy of the package predated
the module that reads them. Files went to local disk with the
configuration sitting perfectly right beside them.
Two shapes are cast to objects deliberately. An empty PHP array encodes as
[], so an installation with no packages -- or holding no files -- would
answer a map-shaped field with a list, and a reader unmarshalling it
breaks on the day it happens to be empty rather than the day it is
written. There is a test for each.
Requested by the ProjectSend Cloud control plane, whose storage figure
stops growing the moment a tenant's uploads start reaching the bucket.
FoldersController::destroy() authorized delete on the folder and nothing else, while FolderService::delete() soft-deletes every file in the subtree and File's deleted hook takes the bytes off disk. So a staff member refused a file one route over could destroy it by deleting the folder around it -- permission and library boundary both unasked.
MyFoldersController::destroy() already draws this line for the client half of the same cascade, and says why: owning the folder is not authority over content someone else put in it. This is the staff half of that sentence.
Verified before merging: the four bug tests fail on main and pass here, and the SQL predicate was read line by line against FilePolicy::delete -- it is a faithful negation, including the null-uploader case and the short-circuit for an unscoped viewer holding both delete permissions. Membership of the check is one COUNT, not a policy call per file. Suite at 2099, PHPStan clean.
Behaviour change, deliberately accepted: a folder delete that used to succeed now refuses, naming how many files are in the way. The likely case is somebody who owns a folder another account uploaded into. The alternative is irreversible loss of files the same person is refused individually.
Not taken: deleting what the actor may and keeping the rest. Half a tree is worse than either answer. Naming the blocking files would be friendlier than counting them and is worth doing later -- the list has to hide any file the viewer cannot see, which is its own small design question.
Reported and fixed by @denkfabrik-li.
file_comments.client_context_id is cascadeOnDelete, but users are soft-deleted, so the cascade never fires: the column goes on pointing at a row that is still there while the relation resolves to null. resolveClientContext() branched on the relation, so "this is Alice's conversation" read as "this has no conversation" -- and a null context on a clients comment is the branch every client on the file reads. A staff reply into a departed client's private thread became a circular, and canAssignClient() was skipped on the way.
That is the invariant docs/feature-comments.md calls the rule everything hangs off: a clients comment carrying client_context_id = C is never returned to any non-staff viewer other than C, because one customer learning another exists is worse than leaking a comment's text.
Verified before merging: both new tests are red on main and green here, and the three that must not move stay green either way. Suite at 2093, PHPStan clean.
The second half is the same root cause through the other column. authorName() read a deleted client's comment as "Anonymous", which is what a visitor's comment looks like -- and a visitor's comment is governed by different rules, so the two must not be able to look the same. Whether the author is a visitor is now decided by author_id alone, the question isFromGuest() already asks.
Accepted consequence: a soft-deleted client's name is visible on their old comments during the erasure grace period, where it previously read as Anonymous. It goes for good when erasure removes the row.
Reported and fixed by @denkfabrik-li.
ProfileController::destroy() validated the current password and soft-deleted, without asking guardLastAdministrator() -- the rule the other four doors ask, at the one door where the account being removed is certainly signed in. The sole administrator could empty their own installation, and EnsureSetupIsComplete, which asks exists() and so skips trashed rows, then handed the first-run setup form to whoever loaded the page next. That form creates an active System Administrator, unauthenticated.
Verified before merging: on main the sole administrator's self-deletion succeeds and setup reopens; both new tests are red there and green here. Suite at 2088, PHPStan clean.
Two locks, because one of these questions is asked at five doors and the other at one. The guard closes the door. And "has this installation been set up" stops meaning "does it have a working administrator right now" -- a trashed staff row is still evidence that setup happened, counted now in both the middleware and SetupController::setupIsComplete(), which have to agree or the result is a redirect loop or an open form.
Worth recording: erasure force-deletes a self-deleted account after its grace period, so the second lock would expire on its own. It does not matter because the first lock stops the installation reaching that state, but a future change to either should know the other is not permanent.
An installation that has already lost its last administrator now finds setup shut. That is the point: recovery is php artisan projectsend:admin, which is also how every unattended container installs itself.
Reported and fixed by @denkfabrik-li.
The job walked the loose file ids and then every selected folder's subtree, adding whatever each pass found. A selection reaching the same file both ways got it twice: two copies of the same bytes, a total_size inflated by the repeat -- which is what the size cap is checked against -- and a file limited to a single download handed over in three copies while the log recorded one, because delivery logs per contained file and DownloadAllowance counts those records.
Verified before merging: the three new tests fail on main and pass here. Suite at 2082, PHPStan clean.
Two halves, because one fix does not cover both shapes. The added-ids list becomes a map keyed by id and the folder pass skips what is already in, before the per-file re-checks, so a duplicate does not spend an allowance twice either. And a folder sitting inside another selected folder is dropped before either is walked, which also settles which path the surviving entry keeps rather than leaving it to row order.
One measured cost, accepted: the pruning compares every selected folder with every other. The pathological case -- ten thousand sibling folders, the selection cap -- benchmarks at around twenty seconds of CPU, in a background worker, on a selection that would take far longer to compress. A sort-by-path-length version would be cheaper if it ever matters.
Reported and fixed by @denkfabrik-li.
Every group route asked StaffLibraryScope whether this viewer may act on this group except the two that read it. So a client-scoped staff member could open the edit screen of a group they cannot change, read its membership with addresses, and get the whole client roster in available_clients besides. The API twin returned the same membership.
Verified before merging: the three new tests fail on main and pass here. Two things checked beyond the report -- group membership is edited through separate, already-guarded routes, so narrowing the displayed list cannot remove anybody on save; and scramble:export regenerates byte-identical, as claimed. Suite at 2078, PHPStan clean.
The fix has two halves because one guard does not cover both shapes. Reading the group now asks the same reach question the write half asks. And both lists narrow through StaffLibraryScope::clients(), because a group nobody has shared anything with reaches nowhere, stays open to everybody, and can still hold a stranger's client.
Unscoped viewers are unaffected: clients() returns the whole roster for them and allowsGroupChange() is true by construction.
Reported and fixed by @denkfabrik-li.
The validation rule accepts 0 and "0" as well as false and does not cast, so a strict comparison against the validated array let two of the three spellings past the self-deactivation guard -- and the model's own boolean cast then stored exactly the value the guard had just decided was not a deactivation.
Reproduced on main before merging: {"active": false} is refused, {"active": 0} and {"active": "0"} both return 200 and switch the account off. Green on the branch, suite at 2074, PHPStan clean.
The fix reads the flag once with Request::boolean() and gives that same value to the guard and to the write -- the rule RolesController::guardScopeRemoval already documents for the same reason. Validation is unchanged, so the accepted inputs are the same; one of them just stops meaning two different things on its way through the method.
Follow-up for the release: this is a caller-visible change (200 to 422) and wants a line in api-changelog.md.
Reported and fixed by @denkfabrik-li.
The test named for carrying the durability verdict to the system widget asserted only has('system'), and system is an unconditional key of the render array -- the controller's own comment beside storage_durability says as much. So the assertion could not fail.
Confirmed here by deleting the line that supplies the verdict: the new assertion fails with "Property [system.storage_durability] does not exist", where the old one stayed green.
Test-only, no application code.
Reported and fixed by @denkfabrik-li.
EnforceTwoFactor exempts by route name, and only the GET half of the confirm-password screen had one -- Route::named() answers false for a null name, so the submission was never exempt. Enrolling requires password confirmation, so with enforcement on nobody could enrol at all: the form rendered, its POST was redirected to two-factor.show, auth.password_confirmed_at was never written, and every account on the installation was left with logout as its only working route. Including the administrator who turned the setting on.
Reproduced on main before merging: POST /confirm-password redirects to /settings/two-factor and the session flag stays unset. The widened pattern was checked against the route table -- password.confirm* reaches password.confirm and the newly named password.confirm.store and nothing else; password.reset, password.store and the rest are not under that prefix. Exempting the submission grants nothing further, since every other route stays bounced and store() still validates the password.
Reported and fixed by @denkfabrik-li.
Ten tests ran the real projectsend:update, which runs clear-compiled, which deletes bootstrap/cache/packages.php and services.php -- one copy for the whole checkout, shared by all eight workers of a parallel run. A worker booting in the window between that delete and its own rebuild reads an empty package manifest, registers no package service providers, and dies rendering the next page with "Target [Inertia\Ssr\Gateway] is not instantiable", in a file that has nothing to do with updates.
Verified here rather than taken on trust: a probe running the real update inside a test on main deletes the manifests, exactly as described. The branch is green at 2066 with PHPStan clean, and touches no application code.
The file already owned a double and explained why the artisan call is a seam; this extends it to the whole file and adds a test asserting the compiled caches survive.
Reported and fixed by @denkfabrik-li.
Two doors onto the client seat cap did not ask it. Both update()
methods -- the edit screen and PATCH /api/v1/clients/{id} -- clear
account_requested when a pending client is activated, under a comment
saying that counts as approval, and approval is the moment a seat is
spent. So a managed installation sitting at its cap kept taking clients
on for as long as registrations arrived, and self-registration is open
to strangers, so the supply of pending rows is not the operator's to
control.
Verified rather than taken on trust: the two new door tests were run
against the unguarded controllers and fail there, and every place in
app/ that clears the flag was enumerated to check no third door was
missed. There is none -- the other six already ask, and a conversion
refuses a pending account outright rather than approving it sideways.
The guard sits inside the approval branch, so an installation at its cap
can still rename a client it already holds. That is pinned by a test of
its own.
Conflicted with tonight's seat work in SeatAllowanceTest, which had
added an import beside the one this adds. Resolved by keeping both;
suite green at 2065 and PHPStan clean after resolution.
Reported and fixed by @denkfabrik-li.
The tests workflow has not parsed since c05927c1 added a second top-level `concurrency:` key four lines below the one that was already there. YAML refuses a duplicate key, so GitHub created a run and scheduled no jobs -- verified here with symfony/yaml ("Duplicate key concurrency detected at line 66") and against the run list: every run since is zero-job, including the commit v2.2.0 is tagged at and all five pushed tonight.
The linter workflow carries one block and kept running, which is why the tree read as checked when the suite had not run at all.
Reported and fixed by @denkfabrik-li.
The docblock argued for one definition by describing a control plane
showing "2 of 3 seats used" next to an application refusing the fourth,
and the two disagreeing. That was written as a thing to avoid. As of
today it is a screen: the hosted fleet console reads these numbers per
tenant out of projectsend:status --json.
Which makes two rules here load-bearing somewhere nobody editing this
file would think to look -- a deactivated staff account still holds a
seat, a client awaiting approval does not. Changing either changes what
a support person is told before it changes what a customer hits, and
the note is here so that is a decision rather than a surprise.
The quick-start list gates its "Add the rest of your team" step on
Capability::UsersManage, which is right and unchanged: it is the seam an
edition difference would travel through. The comment above it still gave
the old reason -- that a managed installation has no staff accounts of
its own to hand out -- which the capability opening on both editions
made false. The step has appeared on a managed installation's list since
623ad68, and GettingStartedTest already says so.
Found by sweeping every repo for the same claim after four others turned
up: core, both module packages, the migration tool, the customer portal
and the private docs. The remaining ones are in the portal's own
planning documents, which are its to correct.
A platform can see that an installation is running. It cannot see
whether anybody is still using it, and the difference is what separates
a customer from an abandoned free instance holding a database.
So the status probe gains one field:
"activity": { "last_staff_login_at": "2026-08-24T21:13:32+00:00" }
Null means no staff account has ever signed in, and the key is emitted
either way. That is the whole care in this change: "they said never" and
"we got no answer" have to stay distinguishable, because collapsing them
is how a broken probe reads as a dormant fleet.
Only interactive sign-ins count. Laravel's Login event does not fire for
token authentication, so an integration polling every hour cannot make
an empty installation look busy -- which matters when the reading is
used to decide something.
Derived from the activity log rather than denormalised onto users. A
column would cost a migration, a listener change and a backfill to save
one indexed MAX() over a table with a handful of rows on exactly the
installations anybody asks this about. Nothing prunes the log, and
erasure anonymises entries rather than removing them -- actor_type
survives on purpose -- so the answer does not change when the person who
gave it is forgotten.
Requested by the ProjectSend Cloud control plane, which has no other way
to learn the date. Recorded in docs/api-todo.md as deliberately a
command rather than an endpoint, for the reason the command exists at
all: it observes, it does not accept instructions.
It stopped being true in 623ad68, when users.manage opened on both
editions. The code moved and these did not, which is the worst kind of
comment: confidently wrong, and about the very rule a reader comes to
them to learn.
PlatformManaged claimed the tenant's own /users screens stay closed,
directly contradicting the UsersManage comment eleven lines above it.
routes/web.php said the same about the group it gates. The API
controller's docblock opened with "**Community only.**", and the
conversion screen's said a managed installation creates staff accounts
elsewhere.
Each now says what is actually true, and says the division the change
turned on: a platform sells the seats, the tenant decides who sits in
them. What limits a managed plan is the seat cap, not a shut door -- so
the API answers 422 at the limit rather than 403, which is a different
sentence to whoever is reading it.
On a managed installation with its staff seats full, /users/create opened
as though there were room. You typed a name, an address and a password
you had to invent, pressed Save, and the plan limit came back as a
validation error under the email field -- which reads as a complaint
about the address rather than a fact about the plan.
A full installation is an ordinary state on a plan sold by the seat, so
it is now stated up front. The list carries the seat position, the
button goes dead once the last seat is taken and says why, and the
create screen turns away anyone who reaches it by link or bookmark. The
guard in store() is untouched: that is still the rule, this is only the
door.
The refusal is worded once, in SeatAllowance, and the screen is handed
that sentence rather than writing its own -- two wordings of one limit
is how somebody ends up believing there are two limits. `full` is
derived there too, from the same comparison the guard refuses on, so a
screen cannot disagree with it about the edge (used > limit, after an
operator lowers a limit) and offer a button for a form that cannot be
submitted.
Clients get the same treatment: the cap exists there too, and reached it
the same way. Self-hosted installations have no limit, so they are shown
nothing about one.
file_comments.client_context_id is cascadeOnDelete, but users are
soft-deleted, so the cascade never fires: the column keeps pointing at a
row that is still there while the Eloquent relation resolves to null.
resolveClientContext branched on the relation, and a null context on a
Clients comment is the branch every client on the file reads -- so a
staff reply into one client's private thread became a circular to all of
them, with the canAssignClient check skipped on the way.
VisibleCommentScope says so in its own docblock: "A Clients comment
carrying client_context_id = C is never returned to any non-staff viewer
other than C ... A Clients comment with a null context is a staff message
to everyone on the file, and every client with access reads it."
Measured on main, with one file shared with two clients and the first of
them deleted after commenting:
column client_context_id 3
relation clientContext null
POST reply into her thread 201, stored with client_context_id null
read by the other client yes
Ask the column, and refuse when the account behind it is gone. There is
nobody left to answer, and the one outcome that must not follow from a
filled column is the broadcast, so this throws rather than falling
through to it.
authorName() had the same root cause from the other column: its docblock
claimed author_id cascades so there is no deleted author, and a deleted
client's comment was going out as "Anonymous" -- which is what a guest
comment looks like, and a guest comment is read by different rules. Guest
is now decided by author_id alone, the same question isFromGuest() asks,
and a trashed author is read with withTrashed(). Nothing comes back only
once the grace-period erasure has removed the row for real.
That read costs one query per comment whose author is trashed. Measured
on a ten-comment thread: 11 queries before, 21 after, against 20 for the
same thread with every author alive. Left as a lazy read rather than
eager-loading with withTrashed() at every call site, because the callers
would each have to remember it and the cost only applies to comments
whose author is gone.
Five tests, two measured red against the unfixed code (2 failed / 3
passed) -- one per column. The three that stay green either way are the
branches that must not move: a staff message with no context still
reaches everybody, a reply into a live client's thread still lands in
that thread alone, and a genuine guest comment is still anonymous.
Full suite passes (2053 passed / 2 skipped), PHPStan level 8 clean.
ProfileController::destroy() validates current_password and soft-deletes.
It never asks StaffAccounts::guardLastAdministrator(), and every other
door does: Staff update(), guardDeletable(), and both directions of the
role conversion. This is the one door where the account being removed is
certainly signed in.
An installation with a single administrator therefore had a button that
emptied it. Measured on main:
DELETE /settings/profile 302, the account is gone
live staff rows 0 (the row is trashed, not removed)
anonymous GET / 302 -> /setup
anonymous POST /setup a new active System Administrator
EnsureSetupIsComplete asks ->exists(), which excludes trashed rows, and
routes/web.php registers GET and POST setup with no auth and no guest
middleware -- correctly, since a fresh installation has nobody to
authenticate. SetupController::store() re-checks the same condition, so
both halves agreed with each other and both were wrong once the last
staff row was trashed.
Two locks, because one of them is asked at five doors and the other at
one.
First: destroy() now asks guardLastAdministrator(), the same call with
the same message as everywhere else. An administrator with a colleague
still goes, a non-administrator staff member still goes, and a client
still closes their own account.
Second: "has this installation been set up" is not the same question as
"does it have a working administrator right now", and only the first one
belongs in EnsureSetupIsComplete. A trashed staff row is still evidence
that setup happened, so it now counts -- in the middleware and in
SetupController::setupIsComplete(), which have to agree or the result is
either a redirect loop or an open form.
That second lock holds even if a future door forgets the first one.
Measured with the guard bypassed entirely and the row trashed directly:
GET / answers with the login screen and POST /setup creates nothing.
Worth stating plainly: an installation that has already lost its last
administrator will now find setup shut rather than open. That is the
point -- the recovery path for it is `php artisan projectsend:admin`,
which is also how every unattended container installs itself, not a form
that anybody on the internet can reach.
Six tests, two measured red against the unfixed code (2 failed / 4
passed) -- one per lock. The other four are the boundaries: a colleague
present, a staff member who is not an administrator, a client, and a
genuinely fresh installation that must still reach setup.
Two existing tests needed saying more clearly rather than changing:
ProfileUpdateTest's deletion cases now create a second administrator, so
that what they assert is self-deletion and not this new refusal; and
GettingStartedTest's "fresh installation" cases forceDelete rather than
delete, because a soft-deleted staff row is no longer a fresh
installation -- which is the whole of the second lock.
Full suite passes (2054 passed / 2 skipped), PHPStan level 8 clean.
BuildZipDownloadJob walks the loose file ids and then every selected
folder's subtree, and adds whatever each pass finds. A selection can
reach the same file from more than one of them, and nothing noticed:
file_ids [f], folder_ids [Reports]
-> ['report.pdf', 'Reports/report.pdf']
file_ids [f], folder_ids [Reports, Reports/Q1]
-> three entries, file_count 3, total_size three times the file
Two copies of the same bytes in one archive, and total_size is what the
size cap is checked against, so a selection could also be refused for a
weight it does not have.
The one that costs more than bandwidth is delivery. It logs one
FileDownloaded per contained file, and DownloadAllowance counts those
records -- so a file limited to a single download left in three copies
while the log recorded one. Measured: three entries, one record.
Two causes, so two halves.
`$added` is now keyed by id instead of being appended to a list, and the
folder pass skips a file already in the archive. A lookup rather than a
scan because the selection cap is 10000 sources. The loose pass runs
first, so a file picked both ways sits under its loose name; either
answer is defensible, but it has to be the same one every run.
And a selected folder inside another selected folder is dropped before
either is walked. Zipping both would reach every file in the inner one
twice, and which path the surviving entry ended up under would be decided
by the order the rows came back in. Keeping the outer folder keeps the
fuller path -- Reports/Q1/report.pdf rather than Q1/report.pdf.
Containment is decided on the materialized path, so it is one comparison
per pair with no queries: a folder's path starts with an ancestor's
subtreePathPrefix(), and both end in '/', so /5/ cannot match /50/.
Not changed: the per-file re-checks inside the folder pass. Visibility
and the download allowance are still re-derived per file, and the skip
happens before them, so a duplicate never spends an allowance twice
either. Nor the selection endpoint -- a caller may send whatever
selection they like, and the job is where it is resolved.
Four tests. Three measured red against the unfixed job (3 failed / 32
passed): the loose-plus-folder case, the nested-folder case, and the
three-way case asserted through delivery rather than through the archive.
The fourth -- two selected folders that merely share a name are both
zipped -- is green either way and guards the pruning against being about
names rather than containment.
Full suite passes (2052 passed / 2 skipped), PHPStan level 8 clean.
Every other group route asks StaffLibraryScope whether this viewer may
act on this group. GroupsController::update() and ::destroy() do, and so
do their API twins -- all four with abort_unless(allowsGroupChange, 404).
The two that read do not: edit() and Api\GroupsController::show() had no
boundary at all.
What they hand over is the membership, name and email per member, plus
the whole client roster of the installation as available_clients. So a
client-scoped staff member could open a group whose contents they cannot
see, read off every client on the installation, and only be refused when
they pressed save.
Two halves, because the leak has two shapes:
- The group itself. Reading it now asks the same reach question the write
half asks, one step earlier, with the same 404 -- a group that reaches
past the viewer's library is not theirs to open either.
- The lists inside it. Both narrow through StaffLibraryScope::clients(),
the listing half of the rule this screen's buttons are already guarded
with: allowsGroupMembership refuses removing a member outside the
roster, and refuses adding a client outside it. Naming them anyway,
with their address, is the mistake ClientsController made before
clients() existed -- that method's own docblock says so.
The reach guard alone would not have been enough. A group nobody has
shared anything with reaches nowhere, so it stays open to everybody --
and it can still hold a stranger's client. That case is why the lists
narrow separately, and there is a test for it.
members_count is left whole on purpose: a size is not an identity, and it
is the same number the group listing already reports.
GroupResource's docblock claimed members are safe to expose because "the
group edit screen already shows [them] to anyone holding edit_groups".
That was a claim about a screen, and it stopped being true the moment the
screen narrowed. Reworded to say what now holds it up, and where.
Not changed: the group listing. It reports names and member counts, not
identities, and every button on it is guarded. Nor Api\GroupsController::
index(), for the same reason. Nor the API document -- scramble:export is
byte-identical, because GET /groups/{group} already documented a 404.
Four tests. Three measured red against the unguarded controllers (3
failed / 21 passed): the group cannot be opened at all, the edit screen
stops naming strangers, and the API twin narrows what it hands back. The
fourth -- an unscoped viewer keeps the whole roster and every member -- is
green either way and guards against the fix over-refusing.
Full suite passes (2052 passed / 2 skipped), PHPStan level 8 clean.
Api\UsersController::update() compares the validated value strictly:
if ($user->is($actor) && ($validated['active'] ?? true) === false) {
The `boolean` rule accepts 0 and "0" as well as false, and it does not
cast. `0 === false` is false, so the refusal never fires -- and the
model's own `boolean` cast then stores as false exactly the value the
guard had just decided was not a deactivation.
Measured against main, with a second administrator present so that
guardLastAdministrator is not what answers:
{"active": false} -> 422, still active
{"active": 0} -> 200, active is now false
{"active": "0"} -> 200, active is now false
The method's own docblock says it is "Refused with a 422 if the change
would leave the installation with no active administrator, or if you
would be deactivating yourself", and the web screen does refuse. This is
the API half of that sentence.
RolesController::guardScopeRemoval documents the rule this breaks, in the
same words: callers resolve the flag with Request::boolean() and hand the
same value to the guard and to the write, deliberately, because reading
the validated array and comparing it strictly "would let a request
through here that the model's `boolean` cast then stores as false anyway
-- the guard and the write disagreeing about one value is exactly the
shape this guard exists to prevent".
So read it once, with Request::boolean(), and give that one value to both.
Not changed: the validation rule. It stays `boolean`, so the accepted
inputs are the same as before -- what changes is that one of them stops
meaning two different things on its way through. Nor anything about
deactivating somebody else: all three forms still work, and there are
tests saying so.
Six cases from two datasets. Two measured red against the unfixed
controller (2 failed / 4 passed): 0 and "0" on yourself. `false` was
already refused, and the three "somebody else" cases are green either way
-- they guard against the fix over-refusing, not against the bug.
Full suite passes (2054 passed / 2 skipped), PHPStan level 8 clean.
The test named for carrying the verdict to the system widget only
asserted that the 'system' key exists. It is an unconditional key of the
Inertia::render array and is allowed to be null, and Inertia's has() is a
key check, so the assertion held whether or not the verdict was in there.
Deleting 'storage_durability' from DashboardController::systemInfo() left
the file green.
Substitute the class the way the rest of the file already does and assert
the payload, as InstallationKindTest does for install_kind next door.
`UpdateWelcomeTest > staff who may not read system information are not
interrupted` fails on a parallel run roughly one time in six, with
BindingResolutionException: Target [Inertia\Ssr\Gateway] is not
instantiable
in a file that has nothing to do with updates. Run alone it is green
every time. The cause is not in that file.
`clear-compiled` deletes bootstrap/cache/packages.php and
bootstrap/cache/services.php. There is one of each for the whole
checkout, and `pest --parallel` gives eight worker processes the same
one. Instrumented over three full runs, the real command ran 12 times per
run -- 11 from UpdateCommandTest, 1 from StaleCodeNoticeTest -- and the
other workers observed the package manifest missing at boot 46 times.
What that costs is in PackageManifest::getManifest():
if (! is_file($this->manifestPath)) {
$this->build();
}
return $this->manifest = is_file($this->manifestPath) ?
$this->files->getRequire($this->manifestPath) : [];
A worker that loses the second is_file() to another worker's unlink gets
`[]`: no discovered packages, so no package service providers, so
Inertia's is never registered and `Inertia\Ssr\Gateway` is never bound.
The next page it renders dies in the compiled root view, where
`@inertia` resolves that interface. Any test in any file, whichever one
happened to be booting.
Both halves measured. Building the manifest with inertia-laravel in
`dont-discover` reproduces the reported failure exactly -- same test,
same exception, same frame (`app('Inertia\Ssr\Gateway')` from the
compiled app.blade.php). And 12 real `clear-compiled` calls per run is
the count above.
UpdateCommandTest already owns a double for this, and says why in its own
docblock: the artisan call is a seam. Nine of its tests and one in
StaleCodeNoticeTest simply do not use it. None of them asserts that a
command ran -- they assert EnsureSystemRoles, the settings writes, the
activity log and the welcome marker, and the double touches none of
those. So the seam now covers the file, through a beforeEach rather than
per test, because the next test added here should not have to know any of
this.
The double moves to tests/Support and its helper to tests/Helpers.php,
for the reason that file documents: Pest hands whole files to workers, so
a class declared in one test file does not exist for another.
Not changed: UpdateInstallation. `clear-compiled` belongs in a real
update. Also not changed: giving each worker its own bootstrap/cache
through APP_PACKAGES_CACHE and friends. That would make the destruction
cheap rather than remove it, and nothing in the suite needs those
commands to run at all.
One new test, on the files rather than on the recorded call list -- a
future double that forgot to intercept one command would still satisfy a
call-list assertion. Counter-checked: with the beforeEach removed it goes
red on both manifests being gone (1 failed / 22 passed).
Eight consecutive parallel runs green after the change; the manifests'
mtimes are untouched by a full run, where before they were rewritten
every time. Full suite passes (2049 passed / 2 skipped). PHPStan level 8
clean -- it analyses `app` only, so it does not cover this change.
Pre-existing and left alone: pint reports `ordered_imports` on
UpdateCommandTest.php. Its import block is misordered on main too.
FoldersController::destroy() authorizes `delete` on the folder and nothing
else. FolderService::delete() then soft-deletes every file in the subtree,
and File::booted()'s `deleted` hook takes the bytes off disk. There is no
restore.
FilePolicy::delete asks two questions the folder route never reaches:
`delete_others_files` for somebody else's upload, and
StaffLibraryScope::allowsFile on top of it. Measured with a role holding
create_own_folders, delete_files, upload and edit_files -- the shape the
Client Manager system role already has, minus delete_others_files:
DELETE /files/{someone-elses} 403, the file is still there
DELETE /folders/{their-folder} 302, the file and its bytes are gone
MyFoldersController::destroy already refuses the client half of this exact
cascade, and says why: "Owning the folder is not authority over content
someone else put in it... Refuse rather than silently destroy them." This
is the staff half of the same sentence.
Counted rather than asked per file. A folder can hold thousands, Gate
resolves a fresh policy for every check, and a per-row policy check on a
listing is the cost 0a8b609e went to some trouble to remove. Both halves
of FilePolicy::delete are expressible in SQL: the permission half is
constant for the viewer, and the library half is the query
StaffLibraryScope already memoises per request. Somebody holding both
delete permissions with no library scope short-circuits before the query
runs at all, so the common case pays nothing.
Not changed, deliberately:
- The service. FolderService::delete stays dumb. Its other caller applies
the client rule ("files you did not upload"), which is a different
predicate, and putting both in one place is the drift this codebase
keeps refactoring away from.
- The client half. MyFoldersController is already correct.
- Nothing partial. A blocked folder is left whole rather than emptied of
what the actor may delete -- half a tree is worse than either answer.
Worth saying plainly: this is a behaviour change. A folder delete that
used to succeed now refuses, and somebody will notice. The alternative is
irreversible loss of files the same person is refused one route over.
Six tests. Four measured red against the unguarded controller (4 failed /
2 passed), one per half of the predicate: the permission half, its
message, a nested file, and the library half -- that last one with both
delete permissions held, so only StaffLibraryScope can refuse. The two
that stay green either way are the other side of the question -- that a
folder holding only your own files still goes, and that an administrator
holding both permissions is unaffected. They guard against the fix
over-refusing, not against the bug.
Full suite passes (2054 passed / 2 skipped), PHPStan level 8 clean.
The new string is English only, per CONTRIBUTING.md -- translations are
their own pass.
SeatAllowance says a cap is only a cap if every door asks, and has a test
per door for that reason. Two doors do not ask.
The moment a seat is spent is the moment `account_requested` is cleared.
Five places do that. approve(), both store()s and ClientProvisioning ask
guardClient(); AccountConversion asks it through guardToClient(). The two
update()s -- web and API -- clear the flag with no guard at all, under a
comment that names exactly what they are doing:
// Activating a pending account through the edit screen counts as
// approval and clears the request flag.
Measured with clients: 0, one pending registration:
POST /account-requests/{id}/approve refused, flag still set
PATCH /clients/{id} active=true approved, clientUsed() 0 -> 1
PATCH /api/v1/clients/{id} active=true approved, clientUsed() 0 -> 1
A managed installation at its cap therefore keeps taking clients on, from
the edit screen or a PATCH, for as long as registrations keep arriving --
and self-registration is open to strangers, so the supply is not the
operator's to control.
Inside the branch, not above it. Above it, an installation sitting at its
cap could not rename a client it already holds, which would trade one
wrong refusal for another. There is a test pinning that.
The field is `active` rather than the default `email`: on this screen the
administrator is toggling `active`, and an error under the email field
would point at the wrong thing. approve() has no form of its own, so it
keeps the default.
Three tests, per door as the file's other eight are. The two door tests
were measured red against the unguarded controllers (2 failed / 18
passed). The third -- that editing an existing client still works at the
cap -- is green either way: it guards against the fix being written a
line too high, not against the bug.
Full suite passes (2051 passed / 2 skipped), PHPStan level 8 clean.
One thing worth knowing that this branch does not touch: on a parallel
run, `UpdateWelcomeTest > staff who may not read...` fails roughly one run
in six on untouched main, with `BindingResolutionException: Target
[Inertia\Ssr\Gateway] is not instantiable`. Measured over 24 baseline runs
before this change existed. It is not this fix, and it is not in scope
here, but it will start being visible as soon as the workflow parses
again.
EnforceTwoFactor exempts by route name, and only the GET half of
confirm-password has one. routes/auth.php:95 names the form
`password.confirm`; :98 registers its submission with no name at all, and
Route::named() answers false for a null name.
So the loop the exemption exists to prevent is still there, one step
further along. With Setting::TwoFactorEnforcement set to staff, clients
or all, an un-enrolled account walks:
GET /dashboard -> two-factor.show
GET /system/settings/security -> two-factor.show
PATCH /system/settings/security -> two-factor.show
POST /settings/two-factor -> /confirm-password (RequirePassword)
GET /confirm-password -> 200, the form renders
POST /confirm-password -> two-factor.show <- not exempt
`auth.password_confirmed_at` is never written, so enrolling can never
start, and every route that is not on the exemption list stays shut --
including Settings -> Security, the one screen that could turn
enforcement back off. Logout is the only door left; recovery is CLI or
database access. It takes one administrator turning the setting on to
reach it, and it reaches every account on the installation at once,
including their own.
The fix is the name. `password.confirm*` then covers both halves of one
screen, matching `two-factor.*` in the same expression; the namespace
belongs entirely to a flow enrolment already depends on being reachable,
and the route table has nothing else under it -- `password.confirm` (GET)
and `password.confirm.store` (POST) are the two it reaches.
Exempting the submission grants nothing further. store() validates the
password, writes a session flag and redirects; the redirect it issues
enters this middleware like any other request, so Settings -> Security is
still answered with two-factor.show after confirming. What changes is
that enrolment can now be started.
Two tests, both measured red against the unfixed middleware: the password
confirmation sticks, and enrolment can be started afterwards (the secret
is written and the screen reports `pending`).
Also named the redirect the existing test settles for. `->assertRedirect()`
with no target passes on this middleware bouncing the request back to
two-factor.show, which is the shape that file exists to refuse. It is a
clarification rather than a guard -- that assertion is green either way,
since the redirect it sees comes from RequirePassword.
Full suite passes (2050 passed / 2 skipped), PHPStan level 8 clean.
c05927c1 added a `concurrency:` block on the premise that the suite never
got one. It already had one, four lines above -- the hunk header of that
diff reads `@@ -50,6 +50,23 @@ concurrency:`, which is the existing block
it was appended below.
A YAML mapping cannot carry the same key twice, so the file has not
loaded since. GitHub still creates a run and then schedules nothing:
553f5fd2 (last green) run 33036453748 jobs=1 ci -> success
d58e4830 (main) run 33114046849 jobs=0 failure
Every run since has that shape, and the run list names it in passing:
those runs appear as `.github/workflows/tests.yml` where the green ones
appear as `tests`, because the `name:` key sits inside the file that did
not parse. `linter` is unaffected -- it carries one block -- which is why
351da21e shows a green linter beside a failed tests run, and the tree
reads as half-checked rather than unchecked.
Reproduced with a parser rather than inferred from the job count:
before -> THREW: Duplicate key "concurrency" detected at line 66.
after -> parsed ok, top-level keys: name,on,concurrency,jobs
Kept the second block, verbatim, because it is the one c05927c1 meant to
end up with and its comment carries the reasoning -- including the
tradeoff that an intermediate commit on `main` can end up with no run of
its own. The two group keys are interchangeable: `github.workflow` is
constant within a workflow, so `tests-${{ github.workflow }}-${{ github.ref }}`
and `tests-${{ github.ref }}` produce the same grouping. Worth knowing
that lint.yml still uses the first shape, if you would rather the two
files read alike.
No test. The failure is loud on the next push, and a test that parses a
workflow file would be a second place to keep the same rule.
It read "limited to 1 staff accounts" -- the number sat directly in front
of a countable noun, which is the message a free-tier customer meets the
first time they try to add anybody.
Adding plural forms would fix English and not much else. Polish, Czech and
Russian inflect the noun by the number in front of it, on a three-way split
that a two-form string cannot express, so ':count kont' cannot be right for
every value however many variants it carries. Ending the sentence on the
number means no language has to agree with it -- the same shape the other
counted strings here already use.
Both strings rewritten in all sixteen locales rather than left to the next
translation pass, since the old key would otherwise go missing and block a
build. Checked at 1 and at 25 in English, Spanish, German, Polish and
Russian.
The summary reads well and says nothing a reader can chase. The numbers and
titles are the way back to the original report, so they go at the end where
they are available without being in the way -- summary at the top for
whoever is deciding whether to upgrade, paper trail at the bottom for
whoever is looking for their own bug.
Generated from the closed-since date rather than hand-picked, and titled
'closed since' rather than 'fixed in' so no per-issue judgement is needed
about how each one was resolved.
Laravel's notification view writes the subcopy URL as [$url]($url).
The HTML half parses that into an anchor; the text half parses nothing,
so it arrives as literal brackets around a duplicated address. With the
button line above it the URL appeared three times in one message.
It reads as broken, and it reads broken in a specific direction: a long
opaque token, the recipient's address in the query string, and a
duplicated link in brackets is the shape of a phishing template. On a
password reset, which is often the first mail an installation ever sends
somebody, from a domain with no reputation yet.
Fixed the way every other component in that message already handles the
same split -- one name, two files, Laravel picks per half. Which meant
publishing the framework's view for a one-line change, so there is a note
in it saying to re-copy on upgrade.
Seen in a real reset mail, not in a test.
The detail underneath was 400 lines of two-and-three-sentence entries.
Written to be complete, and complete is not the same as read: the list at
the top already says what changed, and the long version mostly restated it
at length for somebody who had stopped reading.
The credits do not go with it. Most of the boundary work in this release
came from outside, and dropping the names to save space would be taking
somebody's contribution off the record to tidy a file. One line at the end
instead of twenty inline.
TRUSTED_PROXIES said 'see the fix below' and there is no longer a below.
Written by reading the top of the section and stopping, which is exactly
the failure the list exists to prevent. The fourteen it missed were the
tail: several boundary fixes, the 502 behind a proxy, the recovery code
spent twice.
The near-identical limited-role entries are one line naming the surfaces
rather than six lines saying the same thing, so the list stays scannable.
Same reason as the entry above it. Someone running one installation of
their own has no seats to cap and no provisioning to seed, so three
environment variables they will never set read as noise in the one section
of the file that is supposed to be nothing but things they must do.
The variables and projectsend:status still exist and still work. They are
documented where the people who need them will look.
CHANGELOG.md ships inside the zip and renders in the app, so it is read
mostly by people running ProjectSend themselves. An entry about seats a
host sells you, and screens that were absent on an edition the reader is
not running, is not news to them -- it is somebody else's product notes in
their release notes.
The feature is unchanged; only its account of itself moves.
Twenty-five entries of two or three sentences each is a reference, not a
summary. Somebody deciding whether to upgrade reads the first screen and
stops. The paragraph that was there named four things out of twenty-five
and buried the rest.
One line each, grouped, no jargon. The detail below is unchanged --
it is what you read once the list has told you which entry you want.
An installation brought over from v1 before the migration tool learned to
relabel carries $2a$ or $2b$ digests in users.password. All three bcrypt
labels name the same algorithm and password_verify() reads any of them,
but Laravel's hasher asks password_get_info() first, gets "unknown", and
throws before it looks at the password -- so the login form answers 500
for every migrated account while accounts created in v2 sign in fine.
Relabelling the stored digest is the whole fix. Four bytes change; salt
and digest are the same, so nobody resets anything and there is no mail to
send. Guarded on password_get_info() reading bcrypt afterwards, so a
truncated row is left visibly broken rather than quietly rewritten to no
effect.
$2x$ is left alone on purpose -- it asks for the pre-2011 handling of
bytes above 127, so relabelling it would lock out anybody whose password
is not plain ASCII.
The 500 itself is asserted, not just the repair, so nobody removes the
migration later on the grounds that bcrypt is bcrypt.
Reported by @pabloalvarez44 in #1706.
Asked for by the platform side, and the reason is better than
convenience. Their reconciler's rule is that it observes an end state and
never sends an instruction. `docker exec … php -r '…'` to reach a public
method is an instruction with the caller's argv in it, however harmless
the argv, and it would have been the first crack in that rule. A named
command is an observation, the same kind of thing as reading a directory
size.
`--json` for a machine, plain lines for a person. Nothing here is a
secret or a credential: every field is already visible to any signed-in
administrator, which is what makes it safe to read from outside the
container.
The counts come from SeatAllowance — the code that refuses the account
past the limit — rather than from a second query that agrees with it
today. Two counts that merely agree diverge eventually, over an inactive
account or a soft-deleted one, and the divergence reads as a billing
fault rather than a counting one.
Unlimited is emitted as null, with a test saying so, because the failure
if a reader takes it for zero is a customer on the most expensive plan
whose instance refuses to create a single client. The platform side
independently landed the same care on the emitting end, omitting the
variable rather than sending it empty.
It also answers the question that started all of this. Diagnosing why a
tenant ignored its bucket meant reaching into a container and calling
app() by hand; `projectsend:status` now says which capabilities the
edition grants, which is where that hunt began.
The last of the three. Enforcement is a database setting defaulting to
'none', and on a managed installation the only writers are whoever
administers it and the boot that creates them — so a policy meant to be
on from the start had nowhere to be written. A control plane calling in
afterwards leaves a window between the first account existing and the
policy covering it, and the first account is the one with every
permission.
The entrypoint already seeds an account from the environment. This seeds
the policy one line above it, so the administrator is born under the rule
rather than ahead of it. There is a test for exactly that ordering,
because the ordering is the whole point.
Seeded, never overridden. A value that won on every boot would take the
setting away from the person it belongs to — somebody who tightened it
would find it loosened again by a restart. So it writes only when nothing
has ever been stored, the same shape as `projectsend:admin --if-none`.
Two things that would have been easy to get wrong, both pinned:
'none' is the enum's own default, so Settings::get() cannot tell "stored
as none" from "never stored". Asking the accessor would have overwritten
an administrator who deliberately chose it. The command asks the table.
And it reads config rather than env() directly. `config:cache` stops .env
being read at all, which is how TRUSTED_PROXIES came to have no effect on
any web request while looking correct in the file.
Deliberately not a general PROJECTSEND_SETTING_<KEY> mechanism. Every
setting reachable from outside is one whose value depends on where you
look, and the blast radius of getting that wrong is the settings table.
One named key per setting that needs it.
The three new variables are documented in config/projectsend.php and not
in .env.example or the Docker Hub overview. Those two are written for
somebody running one installation for themselves, and a seat cap is not
a thing they have — FILES_WEB_SERVER_READABLE is in .env.example because
a self-hoster on cPanel genuinely meets that problem.
Opening user management on cloud (623ad68) left a managed tenant able to
create staff accounts without limit. This is the other half, and the two
belong in the same release.
max_clients and max_staff_users are numbers the platform sells and does
not enforce — grep finds them only being passed to screens. The
application is the only process that can count against them, so it
accepts the number from the environment and refuses to exceed it. That is
not the same as inventing a plan tier, which is what config/api.php
declines to do when it will not key a rate limit off billing: nothing
here knows what a plan is.
## One definition
staffUsed() and clientUsed() are public and are what the guards read. A
control plane showing "2 of 3 used" from its own query, beside an
application refusing the fourth from a different one, disagrees
eventually — over an inactive account, or a deleted one — and the
disagreement reads as a billing fault rather than a counting one.
## What counts, and the consequences somebody has to explain
An inactive staff account occupies its seat. Excluding it would make
deactivation a way around the cap rather than a way to revoke access,
since reactivating is one click. The cost is an awkward incentive —
deactivating is the safe removal and keeps paying, deleting frees the
seat and asks what happens to the files — and it is better explained than
hidden.
A client awaiting approval does not. Self-registration is open to
strangers, and counting a pending request would let anybody exhaust a
paid limit from the outside, turning a pricing tier into an availability
control. The seat is spent at approval, which is where the guard sits.
A soft-deleted account frees its seat, though not its address —
AvailableEmailRule holds that until erasure. So a seat can be free while
re-adding the same person is still refused, which is the address rule
rather than this one.
## Eight doors, eight tests
There is no single User::create() to guard. StaffAccounts::create()
covers both staff controllers, but a promotion takes a staff seat without
creating anything, a demotion takes a client seat, ClientProvisioning
serves registration and LDAP and social sign-in alike, and approval turns
an uncounted request into a counted client.
A cap is only a cap if every door asks, so there is a test per door and
each was verified to fail without its guard — eight red, with the two
"must not change" cases green either way. DownloadAllowance's shape for
DownloadAllowance's reason: the failure mode is one of them quietly not
asking, invisible from everywhere except the door that forgot.
projectsend:admin is deliberately uncapped and has a test saying so. It
is the recovery path, and anyone who can run it can also edit the
environment the cap comes from.
A managed installation's staff accounts were expected to arrive from
outside it, so users.manage was Community-only and /users, /roles and
their API twins answered 404 there. The platform side spent a long
document designing its way around that gate; opening it is cheaper than
routing around it, and more honest about where the knowledge sits.
The division that settles it is the one managed storage already uses. We
do not manage a tenant's files from outside — a bucket is provisioned, a
scoped credential handed over, and what goes in it is the tenant's
business. Seats are the same kind of thing. A platform knows how many
staff accounts it sold; it does not know whether Alice should be an
Account Manager, and it certainly does not know where her files go when
she leaves. Capacity is the platform's, occupancy is the tenant's, and
the cap belongs in an environment variable rather than in a closed
screen.
The capability stays in front of the routes rather than being deleted.
It is currently true in both editions, but it is the seam an edition
difference has to travel through, and removing it would mean inventing
one again later.
Seven test files asserted the old rule, which is the tests doing their
job. Most flip. Two needed a different example instead: EnsureCapability
and AbilityCapability were both using users.manage to stand for
"Community-only", so they now use storage.configure and manage_updates —
keys that still are.
Two rationales half-expired and say so rather than being quietly
rewritten. CommentAuthors gave two reasons for being a setting rather
than a permission; the first was that roles are uneditable on cloud,
which stopped being true here, and the second — that `Everyone` includes
anonymous visitors, who have no role to hold a key — was always the
stronger and is now the whole of it.
The seat cap this makes necessary is the next commit, not this one. On
its own this change lets a managed tenant create staff accounts without
limit, which is why the two belong in the same release.
The linter has had this since it was written; the suite, which is the
expensive one, never got it. Two pushes landing together ran two full
suites to the end, and the earlier one was checking a subset of what the
later one checks.
Keyed on the ref, so main and a branch never cancel each other, and a
branch with an open pull request does not fight itself — push and
pull_request arrive under two different refs.
One consequence worth stating rather than discovering: on main an
intermediate commit can end up with no run of its own when two pushes
land close together. That is the right trade when what is being verified
is the state of the branch, but it is not free — a bisect or a release
audit that needs a particular commit's own green tick needs that commit
pushed on its own.
The condition was on the step. A skipped step has still had a machine
allocated for it, and Actions bills per job that runs — so `issue_comment`
firing on every comment in the repository meant every "merged, thank you"
on a pull request, and every comment on an ordinary issue, started a
runner to decide it had nothing to do.
Of the last forty runs, twelve were exactly that. Yesterday's twenty
merges each drew a comment, and each comment drew a runner.
Moved up to the job, where a false condition means no runner at all, and
narrowed with `issue.pull_request` so comments on plain issues stop
qualifying too. GitHub cannot filter `issue_comment` by body at the `on:`
level, so the job is the only place this decision can be made — which is
worth the comment beside it, because the obvious tidy-up is to push it
back down to the step it guards.
Behaviour is unchanged: the same two comment bodies still trigger a
check, and every pull_request_target still does.
Three things went stale, all of them describing the image rather than
selling it, which is the half of that page people act on.
The tag table's worked example was 2.1.0/2.1. The "what is in the image"
paragraph said one queue worker; there are two now, and the second is
there so that building a large zip cannot hold up every notification
email behind it — worth a sentence, since somebody counting processes in
`docker top` would otherwise wonder. And the storage line said local disk
or S3, which stopped being the whole list when Google Cloud Storage
arrived.
FILES_WEB_SERVER_READABLE is new in 2.2.0 and deliberately not in the
environment table. It exists for hosts where nginx and PHP run as
different users, which cPanel and Plesk do; in this image they are the
same user in the same container, so listing it would invite people to set
something that buys them nothing.
Cloud instances are sold seats rather than administering them, so the
tenant's own /users screens stay closed — capability:users.manage is
already Community-only — and a control plane creates, deactivates and
password-resets staff from outside. This is the key that plane gates on.
Only the declaration lives here, the same division StorageManaged and
Branding already use. Everything behind it is a module in the private
cloud-modules package.
Declared before that module exists, deliberately. A capability added
after a release is invisible to every image built from one, and that is
not hypothetical: StorageManaged landed 36 commits after v2.1.0 and has
never shipped, so a fleet with buckets provisioned, credentials scoped
and eight environment variables in place still writes every upload to
local disk — because the gate is here and the gate never left. Declaring
this one now is refusing to make the same mistake twice.
The seat *number* deliberately does not live here. There are no billing
or plan tiers in this application to key off, which is the reason
config/api.php gives for not inventing an installation-level rate limit,
and it holds for the same reason: the number lives where the plans do.
This capability says only who is in charge.
ModuleBoundaryTest grows the other half of its own rule. It filtered on
`api/v1/`, so a package claiming a route anywhere else passed — not
because that was sanctioned, but because nothing was looking, and
/platform/v1 is about to be somewhere else. What it polices now is
machine surfaces, the roots something other than a browser authenticates
to, with api/v1/modules and platform/v1 as the two sanctioned prefixes.
Written twice, because the first version was wrong in a useful way: it
policed every route and immediately caught community-modules' Custom
Assets screens. Those are a module doing exactly what a module is for,
through the host's session and capability middleware in plain sight, and
listing them would be the hardcoded URI list the test above it explains
it is avoiding. Web screens are not the boundary; trusted perimeters are.
Verified by making it fail: a package controller on platform/v2 is caught
and named.
Zip building moved onto its own queue, which a manual install's worker
has to be told about. update.sh repairs the service file and Docker is
unaffected, so the population left is somebody upgrading by hand who
skipped the release note — and for them the failure is the worst shape
available. Email keeps going out perfectly. Zip downloads never finish.
Nothing in any log says why, because nothing went wrong: the jobs sit on
a queue nobody is reading. The person who missed it has no reason to
suspect anything, so the notice has to go looking for them.
The application cannot see its own worker processes, only whether work
gets done, so the question is asked from the other end: was a build
requested that no worker ever picked up? That needs a record of when a
build *started*, which is what the new zip_downloads.started_at column
is — stamped before any of the work, so it says a worker had the row,
not that the row succeeded.
Two conditions, because either alone cries wolf. A build has waited past
five minutes and was never started, *and* no other build is in hand. The
second matters because one worker builds one archive at a time: a queue
behind a large build is a healthy queue, and its waiting rows look
exactly like abandoned ones until you notice something running. "In
hand" is bounded by the job's own timeout, so a worker that died holding
a build stops counting as alive an hour later.
The banner sits beside the stale-code one, on every staff page rather
than the dashboard alone, gated on view_system_info for the reason that
one already argues: a background worker not picking work up is a fact
about the machine, not a feature of an edition. It names the fix rather
than the symptom — "your worker command needs --queue=default,zips" —
because somebody reading that downloads are not being processed still
has to work out what to do about it.
Eight tests, covering both halves of the discrimination rather than just
the happy one: a queue waiting behind a live build stays quiet, and a
build held by a worker that died does not.
Translated into all sixteen locales in the same commit, since a release
is close and a banner nobody can read is worse than none.
Checked on screen as well as in assertions, with a real stalled row on
the dev stack: the banner renders, wraps, and reads correctly.
Everything merged today landed in English, which is the deliberate trade:
a feature never waits on a language nobody in the room speaks. This is
the pass that settles up.
Fourteen keys, sixteen locales, 224 entries. Appended rather than sorted
in, matching how the previous passes left these files, so the diff is
additions and one trailing comma per catalogue and nothing else.
One of the fourteen was a bug rather than a gap. The scoped
expired-files note was written with a `’` escape in the TSX, so the
scanner read the raw source and the runtime read the interpreted string:
two different keys for one sentence, and a catalogue entry for either one
would never have matched the other. The apostrophe is now a literal
character, which is what every other string in these files does.
Checked mechanically — every :placeholder survives, no plural pipe count
moved, `projectsend:erase-account` is intact in the two messages that
name it — and then read on screen, because a file that parses is not
evidence that a sentence fits its button. Settings -> Descargas renders
its label, its paragraph and its help text in Spanish with no overflow.
Orphans left alone at 250. The ten that touch today's subjects were
checked one by one and every one is a false positive of the kind the
skill warns about: `Expired files` is WIDGET_LABELS data, `Uploader` is a
role name from the database, `Page Expired` is laravel-lang's.
Nobody hands out reach they do not hold either
Two resolutions against branches that landed first. #1678 and this one
each add a constructor property and an import to StaffAccounts, so both
are kept. And #1702's merge note called this one exactly: its
"converting an account to staff cannot hand out clients either" case
promoted a stranger client, which #1702 now refuses at 404 before
validation runs. Pointed at a client the actor holds, as that note
proposed, so the request reaches the assigned_clients rule the case is
actually about.
Delete an account and dispose of its content in one transaction
Resolved the conflict with #1678 the way that PR's merge note predicted:
the erasure stamp goes inside the new transaction, so a deletion that
rolls back cannot leave a live account carrying a date on which it would
be erased.
Finalise each chunked upload once, under a per-session lock
Resolved a trivial conflict in ChunkedUploadsTest: this branch and
d7e639b both append tests to the end of the file, so both are kept.
Let a deleted account's email address come back into use
Closes#1648, and with it the last open item of #1647's audit of unique
indexes on soft-deleting tables.
Resolved a trivial conflict in both ClientsControllers: this branch and
today's e7b5b6a each add a constructor property at the same line, so both
are kept. Nothing else overlapped.
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.
Splitting zip builds onto their own queue (92a132d) left manual installs
carrying the one job the release note has to do, and the failure it
produces is the worst shape available: a worker still watching only
`default` sends every email cheerfully and finishes no zip downloads,
with nothing in any log to say why. An upgrade note is a poor place to
put that, because it is read on a laptop and needed on a server.
update.sh already finds projectsend-worker.service, so it now reads the
unit's ExecStart and offers to add --queue=default,zips, keeping a copy
of the original beside it. Before the restart, so the worker comes back
on the command it is going to keep.
Only the unambiguous case is rewritten: a queue:work line with no
--queue at all, which consumes `default` and nothing else. A unit that
already names its queues is somebody's deliberate arrangement, possibly
with a second worker for zips, so that one is described rather than
edited — and one that already includes zips is silently left alone.
Exercised against five unit shapes rather than reasoned about: the plain
command is rewritten and backed up, a declined prompt leaves it untouched
with a warning, a unit already naming zips is a no-op, a custom queue list
without zips warns instead of editing, and a unit that is not queue:work
at all is ignored. The sed itself would double-append if it ran twice;
it cannot, because the --queue= guard above it returns first, and both
read the same first ExecStart line.
The last piece of the #1687 follow-up. BuildZipDownloadJob allows itself
an hour, every shipped topology runs exactly one worker, and everything
shares the default queue -- so one large archive delayed every
notification email queued behind it. The size cap and the
one-build-per-person rule bounded that in July; they did not remove it.
onQueue('zips') in the constructor rather than at the dispatch site, so a
second caller cannot forget it. Both images grow a worker for it:
compose.yaml gains worker-zips, supervisord gains [program:queue-zips],
and the existing worker in each narrows to --queue=default. --tries=1
there matches the job, which records its own failure rather than being
retried.
The part that needs care is the manual install. A worker whose command
still says plain `queue:work` consumes `default` only, so it would send
email happily and never finish a single zip, with nothing in any log
saying why. INSTALL.md's unit now reads --queue=default,zips -- one
worker watching both, which is right for most installations -- and says
what happens if you leave it off, with the two-worker split offered for
anyone who would rather keep the two kinds of work apart. CHANGELOG
carries it as an upgrade note, since it is something to do rather than
something that was done.
Verified in the dev stack rather than only in a test: dispatched a build
and watched worker-zips take it while the default worker stayed idle.
This overturns something #1701 decided, so it should say so. That PR
closed the membership hole and left GroupsController::update and
destroy installation-wide on purpose, on the grounds that managing the
group object is a different question from managing who is in it.
What decides it is a measurement that was not in front of that decision.
An assignment to a group is how its members reach a file, so deleting a
group revokes that access for every member. Measured before this guard,
with a client-scoped role holding the group permissions:
stranger client can read the shared file true
PATCH /groups/{stranger group} 302, renamed
DELETE /groups/{stranger group} 302, group gone
stranger client can read the shared file false
So a staff member who may not add somebody to a group out of their reach
could delete it out from under the people already in it. That is not a
gentler version of the membership rule, it is a harder one, and the two
sitting on opposite sides of the same boundary was the odd part.
StaffLibraryScope::allowsGroupChange is the reach half of
allowsGroupMembership on its own, since no client appears in this
question -- one predicate, two callers, rather than a second statement of
it. Both surfaces take it, at 404, matching the membership guards.
A group that shares nothing beyond the actor's library still passes, so a
group they created or one holding their own clients stays theirs, and
unscoped staff are unaffected by construction.
The API document moves a 404 above a 422 on two paths. Both already
documented the 404 -- route model binding produced one -- and Scramble
orders responses by where they appear in the method, so the guard landing
before the validate() call is the whole of the change.
A full parallel run failed once and passed on retry while I was doing the
#1703 follow-up. A flake is worse than a steady failure: it trains you to
re-run rather than look, and it quietly weakens every green run reported
beside it.
Upload parts are real files under storage_path('app/uploads-tmp/{session_id}'),
not a faked disk. Every parallel worker gets its own database, so session
ids restart at 1 in each of them, and two workers writing parts land in
the same directory. On top of that ChunkedUploadsTest's afterEach deleted
the whole tree rather than its own share, for everybody. Six test files
write parts, so this was reachable without anything I added.
The same collision exists inside one worker: RefreshDatabase rolls back,
so ids restart at 1 for every test, and a run that died before its
cleanup leaves parts sitting under the id the next test is about to
claim.
LocalPartStore now reads its root from config, defaulting to exactly
where it always was -- an installation with UPLOAD_PARTS_PATH unset
behaves identically. Tests\TestCase points it at a per-worker directory
and empties that directory per test, which closes the cross-worker, the
cross-run and the intra-worker versions together. ChunkedUploadsTest's
cleanup and its two directory assertions read the configured root rather
than the hardcoded path, so they can no longer reach into a neighbour.
Verified with eight consecutive parallel runs, green, and by watching the
per-worker directories appear separately (w1, w2, w4 … w14) rather than
one shared tree. The isolation itself cannot be asserted from inside a
single test; what a test can pin is the mechanism it rests on, so one
does: parts go where the configured root says.
Follow-up to #1703, which made `exists:folders,id` mean what its ten
readers already assumed. Two things it named and deliberately left.
**The chunked upload is two requests.** store()'s rule only ever sees the
first: POST /uploads records the resolved folder on the UploadSession and
complete() reads it back from the session rather than from the caller, so
deleting the folder while the bytes are in flight still files the
assembled file into it -- the same orphan state #1703 removes, reached by
a door a validation rule cannot watch. complete() now re-resolves through
Folder::query() and files at the root when the folder has gone.
Root rather than a refusal, because the two moments cost different
things. At store() nothing has been sent, so refusing is free and honest,
which is the call #1703 made. Here the bytes are already uploaded, and
discarding somebody's finished transfer over a folder that vanished
underneath them is the harsher of the two surprises. The file lands
somewhere they can see it and move it.
**The refusal now explains itself.** "The selected folder id is invalid"
says nothing when the answer is that the folder has been deleted -- and
that is the usual way to meet this rule, since a live id picked from a
list is how anybody gets here. It matters most on the chunked path, the
one place #1703 makes a previously-working request fail. A small
ValidationRule object carries the message, which keeps the single
definition Rules::folderId() exists for: a messages() array would have to
be repeated at all ten call sites, and rules meaning different things in
ten places is what went wrong in the first place.
One note for whoever writes the next test here. Upload parts live in
storage_path('app/uploads-tmp/{session_id}'), which is a real shared
directory rather than a faked disk, and each parallel worker's database
restarts session ids at 1 -- so two files writing parts on two workers
collide, and ChunkedUploadsTest's afterEach deletes the whole tree for
everybody. Six test files write parts today. These two cases live in
ChunkedUploadsTest rather than beside the rest of their subject so this
change does not add a seventh racer; the underlying isolation problem
predates it and is worth its own fix.
The other half of the sweep. ClientsController and its API twin checked
`abort_unless($client->isClient(), 404)` and nothing else -- a type
check, not a boundary, which is the phrase #1701 used about the group
membership routes for exactly the same reason.
Measured before the fix, with a client-scoped role holding the client
permissions:
GET /clients every client on the installation, name + email
GET /clients/{stranger} 200
PATCH /clients/{stranger} 302, name actually changed
DELETE /clients/{stranger} 302, client gone
The tell was one route over. ClientFilesController::index already draws
this line with StaffLibraryScope::canAssignClient and calls it "the same
boundary StaffLibraryScope enforces everywhere else in the library". Its
neighbours in the same family did not.
So the predicate is not new here. What is new is StaffLibraryScope::clients(),
the listing half of canAssignClient, so a screen narrows by the rule its
own buttons are guarded with instead of restating it -- restating it is
how this went wrong, and how the last four of these went wrong.
Eight actions take it: edit, update, destroy and the two-factor reset on
both surfaces, plus both listings. Answering 404 rather than 403, since a
client outside the roster should not be distinguishable from one that is
not there -- matching the isClient() guard already above it.
Account requests stay installation-wide on purpose: a self-registered
client who has not been approved belongs to nobody yet, so there is no
roster to narrow by and narrowing would empty the screen.
The published API document is unchanged -- both routes already documented
the 404 that the type check produced.
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.
#1701 closed a real hole: group membership decides what a client reaches,
and through File::scopeVisibleToClient it decides what the staff member
holding that client reaches, so `edit_groups` alone was never a boundary.
The predicate it added asks whether everything shared with a group is
already inside the actor's library.
It asked by counting: pluck the group's assignment rows, count how many
of those ids the library query returns, and require the two to match. An
assignment row outlives the thing it points at — nothing clears them when
a file or folder is deleted — while files() and folders() exclude trashed
rows by construction. So one deleted file left a count that could never
balance again, and the group closed permanently: the scoped staff member
could no longer add their own client to it, or remove anybody from it,
with a 403 and nothing to explain it. Every group accumulates dead
assignments over time, so groups would have gone quiet one at a time.
Asked the other way round — is there anything live, shared with this
group, that is outside my library — the dead rows drop out by
construction, because the query starts from File/Folder rather than from
the assignment. That is also the truer question: a deleted file is not
reach, since nobody can reach it.
Three tests. A group stays usable after a file shared with it is deleted,
including removing a member; the same for a deleted folder assignment;
and the half that must not soften — a live file still out of reach is
still refused, deleted siblings or not.
#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.
#1698 moved the library boundary into FileCommentPolicy, where it
belongs, and said plainly what that cost: the moderation screen went
from 65 queries to 465 for a client-scoped moderator with five assigned
clients. Measured here, those numbers are exactly right.
The cost is not in asking. It is that StaffLibraryScope::files() rebuilds
its query every time, and building one runs four immediate lookups per
assigned client — the client's group ids, the same ids again inside
Folder::sharedFolderIds(), that method's own assignment lookup, and the
shared-folder get() in Folder::scopeVisibleToClient(). None of them
depend on the query being built. Gate resolves a fresh policy for every
check, so a listing paid for all of it once per row.
The built query is now memoised per user and handed back as a clone,
since every caller adds to it, and the scope is registered as `scoped`
rather than transient so the memo survives a request. Scoped rather than
a singleton on purpose: a long-lived queue worker keeps singletons
between jobs, and a library query built from one job's data has no
business answering the next one's question.
That is 465 queries down to 60 on the same page — below the 65 it cost
before #1698, because the memo also helps the callers that were already
asking repeatedly. FileVersions::sharedAudience(), which runs the same
helper twice per candidate while resolving notification recipients, gets
it for free.
So the answer to the question #1698 left open is neither of the two it
offered. can_delete stays a real question asked of the policy; nothing
restates the boundary; and the page is faster than it was before the
fix. Three tests: one user's query never answers another's, one caller's
constraints never follow the next, and the moderation screen does not
ask once per row.
StaffAccounts opens with the rule: "Nobody hands out authority they do
not hold ... that turns one permission into every permission and makes
the rest of the matrix decorative." mayGrant() enforces it for a role's
permissions, guardTarget() applies the same test to an existing account,
and RolesController::guardGrantablePermissions() names the attack in
full -- a non-administrator holding manage_users minting a role that
carries more than they do, and then holding it.
A role carries one more thing, and it is the larger one. `client_scoped`
decides whether the role reaches the clients assigned to its holder or
the whole library, which is the boundary StaffLibraryScope,
ActivityLogScope and every listing in the application are built around.
Nothing weighed it. store() and update() wrote the flag straight from
the request, and mayGrant() looked only at permissions -- so
`manage_users` on a client-scoped role was enough to take the limit off
that role and keep working, or to mint a role without one and move into
it. Either way the next request read the whole library, and the
`assigned_clients` roster that #1697 protects stopped meaning anything
for that account.
Both halves of the existing pair get the missing clause:
- guardScopeRemoval() in RolesController refuses a client-scoped actor
who creates a role without the limit, or takes the limit off one
that has it. Phrased as "removes the limit" rather than "is not
limited", so only what this request changes is weighed -- the same
reasoning guardGrantablePermissions() gives for looking at the diff.
Editing an already-unlimited role's permissions is not this actor
lifting a limit. Both writers resolve the flag with
Request::boolean() and hand that same value to the guard and to the
write: the `boolean` validation rule accepts "0" and 0 as well as
false and validates without casting, so reading the validated array
and comparing it strictly would leave this guard and the model's own
`boolean` cast disagreeing about one value -- which is the shape the
guard exists to prevent.
- mayGrant() refuses a client-scoped actor granting a role that is not
client-scoped, which closes assigning an existing one. It reaches
both surfaces at once: assignableRoleIds() validates role_id on the
web and API staff forms and on the account converter,
assignableRoles() fills the pickers, and guardTarget() covers the
account itself.
Administrators are unaffected -- mayGrant() returns early for them, and
an administrator role is never client-scoped. Unscoped staff are
unaffected: the clause is conditioned on the actor's own scope, so a
non-administrator with manage_users and no limit creates, edits and
grants exactly as before. The seeded roles are untouched; update()
already refused to move the flag on a system role, which is why the
stock Client Manager was never the way in.
Two changes a client-scoped holder of manage_users will notice, both
following from mayGrant():
- the role picker on the staff form and the account converter now
offers only client-scoped roles, rather than offering one the
request behind it would refuse;
- editing or deleting a staff account whose role is not client-scoped
now answers 403, through guardTarget(), on the same "if you could
not grant their role you have no business editing that account"
rule that already applied to permissions.
The roles API is read-only (GET /roles is the whole surface), so this
half has no API twin to mirror; the account half is covered above.
The four routes that edit a group's membership -- add and remove, web
and API -- contain no authorization call of any kind. `can:edit_groups`
in front of them is the whole of it, and a permission is not a boundary.
The authorization sweep looked at these and let them stand, on the
grounds that groups are installation-wide by design: GroupsController
::index lists every group unfiltered, so list and single-object access
agree, and there is no listing/direct-access mismatch to fix. That is
true, and it is the answer to the question of who may *see* a group.
This is a different question: what a write to one *does*.
Joining a group hands the new member everything shared with it. When
that member is one of a client-scoped staff member's own clients,
File::scopeVisibleToClient hands the same content straight back to them
-- that scope is what StaffLibraryScope::files() is built out of. So the
one write turns a file they get a 403 on into a file in their library,
and the download that follows is a 200. ResolvesShareTargets draws that
line on the sharing path through canAssignGroup(); nobody drew it on the
membership path, and canAssignGroup() is *derived from membership*, so
whoever may edit the list also decides what the list entitles them to.
StaffLibraryScope::allowsGroupMembership answers it directly instead of
through the derived predicate, which is the wrong tool here twice over.
Membership asks about reach, so it checks reach: the client must be one
this staff member holds, and the group must not already reach past their
library -- no file assigned to it, and no folder shared with it, outside
StaffLibraryScope. A group nothing has been shared with passes trivially,
which matters, because canAssignGroup() would have said no to a group
that has no members yet and left a scoped staff member unable to put the
first client into one they had just created.
The same write has a second door. MembershipRequestsController::approve
joins a client to a group with identical consequences, under
`approve_groups_memberships_requests`, and deny() decides about somebody
else's client and emails them about it. Both go through the same
boundary, answering 404 to match the guard already above approve().
The queue and its sidebar badge are narrowed to the clients the viewer
holds, through one scope on the model that both read -- the rule the
comment badge in HandleInertiaRequests already states two branches down
("a client-scoped staff member is not shown a number they cannot act
on"), and the reason VisibleCommentScope owns its own pendingTotal()
rather than leaving the middleware to count for itself. Each row carries
the client's name and email, so an unnarrowed queue was also handing
those over for clients outside the roster. Unscoped staff still see every
pending request.
That narrowing is on the client, not on the group: whether a group is
reachable depends on what is shared with it, which is not a question to
ask row by row in a listing. A scoped viewer may therefore still be
shown a request they would be refused on -- one of their own clients
asking to join a group out of their reach. The names were the part that
leaked.
Unscoped staff are unaffected throughout -- both halves of the predicate
are true for them by construction. No seeded role reaches this: Client
Manager is the only client-scoped role that ships, and it holds no group
permissions, so a custom role is needed to get here at all.
The published API document gains a 403 on both member routes.
Regenerated with php artisan scramble:export; Scramble reads abort_unless
out of the method body but not out of a private helper, which is why the
guard is written out at each of the four call sites rather than shared.
approve() refuses a request that is not pending:
abort_unless($group !== null && $client !== null
&& $membershipRequest->status === MembershipRequest::STATUS_PENDING, 404);
deny(), one method below, checks nothing. Denying is not idempotent, so
repeating it is not a no-op:
- denied_at is stamped again, and that is what the client's re-request
cooldown counts from (MyGroupsController::inDenyCooldown). Repeating
the request keeps one client out of one group for as long as somebody
cares to keep asking, without a single new decision being made.
- a second GroupMembershipDenied entry goes into the activity log, for
a denial that did not happen.
- a second "your request was declined" mail goes to the client.
The queue lists only pending requests, so nothing on the screen offers
this; it takes asking for the route directly. It needs
approve_groups_memberships_requests, so it is not a stranger's move.
The guard is the same one, answering the same 404, placed where deny()
can reach it. deny() keeps tolerating a vanished group or client -- that
tolerance is deliberate and separate: the denied row persists for the
cooldown even when the group it named is gone, and index() already
filters those rows out with whereHas.
Not in this change: deny() writes the status, the log entry and the
notification without a shared transaction. approve() has exactly the same
shape, so fixing one alone would replace a symmetry with a difference,
and doing both means also deciding where the mail sits relative to the
commit -- which is the question #1691 answers for file bytes, and worth
answering on its own rather than inside a state-machine fix.
consumeRecoveryCode() reads the whole list, filters the used code out,
and writes the whole list back. Two requests that both read before
either writes each store their own copy, and the second write puts back
the code the first removed. So a spent code comes back, and the same
code offered twice is accepted twice -- while the method's first line
says "each code works exactly once".
Nobody gets in through this who was not already holding a valid code, so
it is a promise not being kept rather than a door standing open. The
promise is worth keeping anyway: it is the whole reason a printed sheet
of recovery codes can be crossed off, and it is what makes a code that
somebody watched being typed in stop working.
The decision now comes from the row as it stands, re-read under a lock
inside the transaction that writes it -- the shape SendNotificationDigest
already uses to claim the rows it is about to delete. A conditional
update, as in PublicShareController's downloads_count and the delivered_at
claim in #1692, is the other precedent in the tree, but the column is
`encrypted:array`: there is nothing in it a database can compare, so the
comparison has to happen after decryption, under something that holds the
row while it does.
The lock is what makes it atomic against a request arriving at the same
moment. The re-read is what makes the decision right, and it is the half
a test can show: SQLite ignores lockForUpdate, so the accompanying tests
pin the re-read and say so rather than claiming to prove the locking.
config/database.php runs MySQL or Postgres in production, and both honour
it. Saving through the caller's own instance keeps that instance in step
with the row, so a caller cannot go on to decide from a list the database
no longer has.
Folder uses SoftDeletes. The `exists` rule runs against the table, so a
folder in the trash passes it -- while every resolution that follows goes
through Folder::query(), which honours the soft delete and finds nothing.
Ten rules across five controllers rely on that check, and each one reads
it as "this folder exists".
Two of them then wrote the id anyway. Api\FilesController::store()
resolves the folder, hands the null to Folder::uploadableBy(), is told
yes -- correctly, that is the rule for a root upload -- and passes
$validated['folder_id'] to the write. FilesController::store() is the
same shape once #1694 gives it the guard. FilesController::update() and
its API twin write it straight through with nothing in between.
The result is a live file inside a deleted folder, which is a state
nothing else in the application produces: FolderService::delete() deletes
every file in the subtree along with it. The row is reachable by id, in
search and over the API, and missing from the listing its uploader would
look in.
Rules::folderId() makes the check mean what its readers assume, once,
where the reasoning can be written down -- the same argument slug() makes
for itself one method above. Every site takes it, so the file cannot end
up with two spellings of the same rule and no way to tell which is the
safe one.
What changes, path by path:
- POST /files, POST /api/v1/files, PATCH /files/{file} and
PATCH /api/v1/files/{file} refuse a folder in the trash instead of
writing its id. This is the fix.
- POST /uploads used to accept it and quietly file the upload at the
root -- its guard and its write already agreed, on null. It now says
so instead, which is what the other upload paths do.
- files/{file}/move, files/bulk-edit, folders, folders/{folder}/move
and the portal's my-folders already refused, through
StaffLibraryScope::folders() or Folder::scopeVisibleToClient(), both
of which drop trashed rows. They still refuse; the answer is now 422
naming folder_id rather than a bare 404. Those two guards are asking
a different question -- "is this folder yours" -- and they keep
asking it.
No live folder id behaves differently anywhere, and the root (a null
folder_id) is untouched.
The published API document is unchanged: `exists` renders the same either
way. Regenerated with php artisan scramble:export and byte-identical.
Every route that binds one client — the edit screen, the update, the
delete, the second-factor reset, the file browser — asks whether this
staff member may manage that client. POST users/convert/{user} binds one
too, and asks nothing about it.
AccountConversion::guardToStaff() says why it skips
StaffAccounts::guardTarget, and the reason is sound as far as it goes:
guardTarget asks "could the actor have granted the target's role", which
is meaningless of a client, and what limits a promotion is the role being
*granted* — enforced by the controller validating role_id against
assignableRoleIds(). That answers the question about the role. Nothing
answers the one about the target.
So a client-scoped staff member holding manage_users, edit_users and
edit_clients could promote any client on the installation. It is the
most far-reaching thing that can be done to a client account: the portal
access goes, the assignments that made them somebody's client go inert,
and they come out holding whatever staff role the actor picked from
their own list. The client is never told.
guardToStaff() now asks StaffLibraryScope::canAssignClient — the same
predicate ResolvesShareTargets uses to decide who a file may be shared
with, and true by construction for unscoped staff, so the ordinary
administrator path is untouched. 404 rather than 403, matching both the
isClient() check the controller makes on the way in and the answer the
clients routes give: a client this staff member may not manage should
not be distinguishable from one that is not there.
guardToClient() is unchanged. Its target is a staff account, guardTarget
is the right question to ask about one, and it was already being asked.
The account list on the converter screen is deliberately left as it is.
It runs under can:edit_users and shows every account of the chosen
direction, the same way every other staff surface that lists clients
shows all of them; narrowing a listing is a product decision, not this
fix. What changes is that the button on the row now refuses rather than
going through.
An account deleted by an administrator was soft-deleted with erase_after
null, so projectsend:purge-erasures — which filters on
whereNotNull('erase_after') — never reached it, and the unique index on
users.email kept the address reserved forever. Anyone re-creating the
account got "The email has already been taken", naming a conflict nothing
on any screen could show or clear (#1648).
Both halves of the issue's option 3:
Every deletion path now schedules the erasure. The stamp lives in
ErasureSchedule — self-deletion switched to it, and StaffAccounts::delete
(shared by the web screen and the API) and both client controllers call
it right before delete(). Same grace period, same purge, whoever deleted
the account. Deliberately no backfill for rows deleted before this
change: stamping them during an update would start a countdown to data
erasure that nobody chose at deletion time; the message below covers
them instead.
The staff creation paths swap unique:users,email for AvailableEmailRule,
which refuses exactly the same things but can explain the one refusal
the stock message can't: an address held by a deleted account now names
the date it becomes available, and one deleted before scheduling existed
points at projectsend:erase-account. A living account keeps the stock
message, and public registration keeps the stock rule — telling an
anonymous visitor the address belongs to a deleted account would confirm
it had an account here.
Folder::uploadableBy() returned true for any staff member without looking
at the folder, on the strength of a comment saying staff had already
validated folder_id through FilesController's own flow. No upload path
did. FilesController::store() did not check the folder at all; the two
that called uploadableBy() — the API upload and the chunked upload the
browser actually posts to — called a guard that could only ever say yes.
A client-scoped staff member could therefore name any folder id and put
the file inside a subtree shared with somebody else's client, where
File::scopeVisibleToClient hands it over without an assignment row ever
being written. That is the boundary StaffLibraryScope's own docblock
claims to hold everywhere.
The staff branch now asks StaffLibraryScope::allowsFolder, which returns
true for unscoped staff, so nothing changes for them. The client branch
is untouched: a client is never client-scoped, and ownership or a public
folder opting into client uploads remains the whole of their rule.
The two folder pickers that fed those ids are narrowed the same way the
listings around them already are.
VisibleCommentScope says at the top of the class that its callers must
already have established that the viewer may see the file. The public
listing's comment endpoint establishes only the guest half of that — the
file is reachable without an account — and then hands $request->user()
straight to the authenticated reading.
For anyone the file's own gate would refuse, that reading is far too
wide. A staff account outside its library, or one holding no file
permission at all, read the file's staff-only notes; a client the file
was never shared with read the messages staff addressed to that file's
clients. Both get a 403 from GET /files/{file}/comments and needed only
to ask the public URL instead.
The endpoint now asks the file's own gate which reading applies. A reader
it admits sees no less than before. A reader it refuses gets what a
visitor gets, widened by their own comments — which is what this
controller has always promised them, and all it promised. The
held-comment rule moves into a method both readings share rather than
being restated.
The same root reaches PATCH and DELETE /comments/{comment}, which bind a
comment rather than a file and so never authorized `view` on it either.
They answer with the thread, and now with the one the file's gate allows.
Refusing them outright would be wrong: somebody who commented through
the public page is exactly the person entitled to edit their own words.
StaffAccounts opens with the rule for roles: "Nobody hands out authority
they do not hold … that turns one permission into every permission and
makes the rest of the matrix decorative." mayGrant enforces it, and
guardTarget applies the same test to an existing account.
The client roster never got the same treatment. `assigned_clients` was
validated as `exists:users,id where type = client` and passed straight to
syncAssignedClients, with nothing anywhere asking whether the actor holds
the clients they are handing out. Assigning a client is not a label: it
is that client's whole library, given to whoever is on the other end.
The case that matters is the actor's own account. guardTarget returns
immediately when the target is the actor — editing your own name and
email is not a question of authority — so a client-scoped staff member
with edit_users could PATCH their own id with every client on the
installation and read the whole library from then on.
assignableClientIds() answers the roster question the way
assignableRoleIds() answers the role one, and the five places that accept
`assigned_clients` — users store and update on both surfaces, and the
account conversion — validate against it. It returns the full roster for
an unrestricted actor, so every caller validates against one list instead
of composing a conditional rule; that list is already client-typed, which
is why one rule replaces the exists() and the type filter together.
The two pickers that offer the roster are narrowed to the same list, so a
form no longer offers a client the request behind it will refuse.
syncAssignedClients is untouched: which ids stick to which role is its
decision and it was never the problem.
A download limit is checked when an archive is ordered and again while it
is built, but it is only spent when the archive is collected. Nothing
about ordering or building moves the count, so every check along the way
sees an allowance that is still untouched.
That turns a prepared archive into a voucher. Order the same limited file
into ten archives and all ten pass, because at the point each one is
checked nothing has been taken yet. Collect them all and the file has
been downloaded ten times against a limit of one. The three endpoints are
independent of the interface that normally drives them, so this needs
nothing more than calling store() in a loop — and no timing luck at all,
since the archives can be collected minutes apart.
DownloadAllowance says of itself that six routes put a file's bytes on
the wire and that every one of them asks, precisely because there is no
choke point to put the rule in. The zip pair asked in the two places that
do not count and not in the one that does.
So the delivery re-checks what the archive holds, where the count
actually moves. Refusing is 403, matching the single-file download route
for the same situation. It is also the only one of the two candidates
that reaches the person: an archive is fetched by navigating to it, and
there is no error view for 422, so the message would be replaced by the
framework's generic "something is broken" page.
One refused file refuses the whole delivery, because nothing can be taken
out of a finished archive without building it again. Ordering the same
selection afresh is the way through — the build leaves the spent file out
and names it in skipped_files, which the poll already reports. This is
stricter than store(), which drops spent files from a selection and
refuses only when nothing survives: there, a selection can still be
narrowed, and here it cannot.
Checking costs nothing where nothing is limited. An unlimited file is
answered from its own column and never reaches a count.
Claiming the delivery is a conditional update now rather than a read
followed by a write. Two fetches of one archive arriving together both
saw delivered_at unset and both wrote a full set of downloads, counting a
single delivery twice — the same shape as the conditional increment that
guards a share link's max_downloads. Only the fetch that moves the column
logs anything; the other still receives the archive, which is the
existing rule that re-fetching one prepared zip is one delivery.
Two things this deliberately leaves alone. Simultaneous downloads of one
file can still both pass before either is logged: that race is documented
in DownloadAllowance, and closing it needs the counter column it explains
why it does not have. And an archive already delivered stays fetchable
for its 24 hours even once the limit is spent — one delivery, re-fetched,
which is what that rule is for.
An archive built before the job recorded its contents is handed over the
way it always was, without this check. What it holds can only be guessed
at by resolving the selection a second time, and guessing is exactly what
must not decide a refusal: the same reconstruction refuses over files the
archive does not hold and misses files it does. Those rows stop existing
within a day or two of an upgrade, and until then they behave as they did
before this change rather than worse.
5754016 moved this controller's thumbnail() and preview() onto
StoredFileResponse and left download(), the last method in the same
class, building its own response:
'X-Accel-Redirect' => '/protected-files/'.$file->path,
That prefix is nginx's internal location for the local files disk, and
$file->disk is never consulted. On an install with external storage
switched on it names a path nothing ever wrote, so the public download
fails — while the same file downloads correctly from the file manager
and from a share link, and previews correctly from this very page,
because all three go through the object that knows the rule.
That commit's own message names the shape: the knowledge "was sitting in
a private method on one class and inline in another, so the next caller
could not inherit it and did not". It is an object now, and this is the
call site that was not moved onto it. StoredFileResponse is already
injected here as $this->bytes — preview(), two methods above, uses it —
and attachment() is the method FileDownloadController and
PublicShareController already call.
Nothing changes for a local install: attachment() emits the same four
headers this method wrote by hand, through the same ContentDisposition
call. The return type widens to Response|RedirectResponse because a
non-local disk answers with a redirect to a presigned URL, which is the
signature preview() already declares.
The regression test fails against the unfixed controller — checked in
both directions rather than assumed. The existing local-disk case grew
assertions for the other three headers, so "unchanged for local" is
pinned rather than argued: it passes before and after.
A zip download's row stores what was asked for — some file ids, some
folder ids — and the download action resolved that selection a second
time, when the archive was collected, to decide what to log as
downloaded.
The two are not the same thing. Folder contents are resolved against the
scope as it stands at that moment, and an archive is written some time
before it is fetched. Add a file to the folder in between and it was
logged as downloaded without ever having been in the zip. Move one out
of the folder and it was handed over without being logged at all. The
same goes for a file that expired or otherwise left the requester's
scope after the build: its bytes are in the archive either way. Nothing
about this is visible to anyone — the download count on the file is
simply wrong.
The job already walks exactly the set that goes in, and already counted
it for file_count. It now keeps the ids rather than a tally, and the
download action logs those. count() gives back the number it was
keeping before.
Rows written before this column existed fall back to resolving the
selection, which is what they were built for; the purge command clears
them within a day.
Follow-up to #1687, which made a zip build report failure honestly. Four
things it passed near, none of them regressions it introduced.
A zip has never had a size limit — only a cap of 10,000 files, which
bounds nothing that costs anything. Ten thousand spreadsheets zip in
seconds; two hundred videos is an hour of stream-copying and an archive
that fills the disk. Bytes are what a build actually costs, so the new
Settings → Downloads screen caps the total size instead, at 2 GB out of
the box. It is a setting rather than a constant because the safe figure
depends on free disk, on whether sources live on a remote disk, and on
the plan a hosted tenant is on — the file count stays fixed, since it is
a foot-gun rail and not a knob anybody needs. The controller measures
the selection at request time and names both numbers when it refuses;
the job measures again, because it re-derives the selection at run time
and a folder can grow while the job waits in the queue.
Every shipped topology runs exactly one queue worker, and everything
shares the default queue, so raising the job timeout to an hour handed
any signed-in person an hour of everyone else's notification mail. There
is now one build in progress per requester and a named throttle bucket
on the endpoint, which had neither. A pending row older than an hour is
treated as abandoned rather than in progress, so a worker killed hard
enough to skip failed() cannot lock somebody out for good. Giving zip
builds their own queue is the structural fix and wants its own change:
it touches compose, supervisord and the systemd unit in INSTALL.md, and
an install that upgrades without changing its worker command would stop
building zips silently.
zip_downloads.requested_by cascades on delete, so removing a user takes
their rows with it and strands every archive they built — invisible to a
purge that walks rows, and to OrphanFileScanner, which skips zips/ on
purpose. The purge now also sweeps files in zips/ that no row explains,
after a day's grace so a build in progress is never taken out from under
itself.
Two smaller things while in here. A build that failed because every file
had already hit its download limit said only that nothing was available,
and dropped the skipped list — the same distinction the store guard goes
out of its way to draw at request time. And a failed close() now logs
libzip's reason, which the @ silencing had been discarding: "the disk is
full" and "the source vanished" are different problems for whoever has
to fix one, while the requester still sees a message with no server
paths in it.
File::booted() removed the bytes the moment a row was deleted. For a
single file that is right. Two paths delete files inside a transaction,
though, and both delete many at once: FolderService::delete() takes a
folder's whole subtree, and DeletedAccountContent::cascadeDelete() takes
everything an account uploaded.
Anything that rolls either transaction back puts every row back while the
bytes are already gone. A transaction exists to make a set of writes
undoable, and removing the bytes was the one write in that set that
nothing can undo. The account path is the sharper one: since content
disposal is nested inside the caller's transaction, the write that fails
need not be in this code at all.
The two failure directions are not equal. Bytes gone with the rows
restored leaves rows pointing at nothing and no way back. Rows gone with
the bytes left leaves orphans on disk, which OrphanFileScanner already
exists to find. Defer to the recoverable one.
Three properties this relies on, all of them checked rather than assumed:
without a pending transaction the callback runs immediately, so a single
delete is unchanged; a savepoint committing inside a larger transaction
does not fire it, which is exactly the account case; and the connection
is the row's own rather than whichever is default.
detachOnDelete stays inside the transaction — it repairs the version
chain's pointers, which is database work that must roll back with
everything else.
MyFilesController::index() builds the portal's folder list in two
branches. At the root it narrows to $visibleIds; one level in it listed
every direct child of the folder being browsed, visible or not.
Opening one was still refused — $current is resolved through
visibleToClient() and 404s otherwise — so what escaped was the name, not
the contents. A name is worth protecting here for the same reason
VisibleCommentScope gives about its own boundary: it is what stops one
customer learning that another exists.
$visibleIds is computed once and wanted in three places. The root branch
narrows by it, the breadcrumb narrows by it, and the nested branch did
not. That is a gap rather than a distinction, and the class docblock had
already promised the opposite: "Group and internal folder names never
leak."
Reachable only where a folder is visible for the created_by reason
rather than by sharing. Inside a shared subtree every child matches by
path prefix anyway, so nothing leaks there. A client with
create_own_folders makes such a folder through POST /my-folders, and
staff can file anything inside it — FoldersController::store resolves its
parent through StaffLibraryScope, which is unfiltered for unscoped staff.
Files were never affected: that branch's query already starts from
File::query()->visibleToClient(). The breadcrumb already trims to the
first visible ancestor. Neither is touched.
update() validated preferences.*.type as ['required', 'string'], so any
string at all became a row in notification_preferences. Nothing reads it
afterwards: emailEnabledFor() looks preferences up by a key the registry
knows, so a row under an unknown key is invisible for good.
It is not a way into somebody else's settings — user_id comes from the
session, never the payload — which is why this is validation rather than
authorization. The cost is a table that quietly accumulates rows nobody
can see, explain, or remove through the interface.
edit() already knew the answer. It filters the registry down to the types
that can email at all, by either route, and renders exactly those as
toggles. That list is now derived once and used by both halves, so what
the screen offers and what it accepts back cannot drift apart.
Rejecting a registered-but-unmailable key (client_uploaded is the one in
tree) is deliberate rather than incidental: FilesServiceProvider explains
that it has no mail companion on purpose, so a preference row for it
could never change what anybody receives.
FileCommentPolicy::moderate() asked only whether somebody is staff and
holds moderate_comments. It never weighed the file the comment sits on,
and delete() returns true the moment moderate() does — so a client-scoped
moderator could delete any comment on the installation by naming its id.
Three call sites already knew this and wrote the boundary out by hand,
each with its own abort_unless($library->allowsFile(...), 403) after the
gate. The two that did not are FileCommentsController::destroy(), web and
API: both bind a comment directly, so nothing earlier in the request
establishes that the viewer may see its file.
The intent was documented in three places and enforced in none of them by
the policy — StaffLibraryScope says "the policies consult allowsFile() so
direct access respects the same boundary", VisibleCommentScope says "a
moderation screen is not a way around the visibility model". Put the rule
where those docblocks already say it lives.
moderate() now takes the comment when there is one. Named against the
class it still answers the coarser "does this user moderate at all",
which is what the queue's gate and the affordances ask. Membership is
tested by file id, so a file soft-deleted out from under its comments is
not in a scoped moderator's library either.
The author branch of delete() is deliberately untouched: deleting your
own words inside the edit window is not moderation, and a client is not
client-scoped in StaffLibraryScope's sense.
Approving through the API now derives its 403 from Gate::authorize rather
than the removed abort_unless, so the committed OpenAPI document gains
the shared AuthorizationException ref in place of an inline "An error"
schema — the shape nine of the other twelve documented 403s already use.
I said they could not, in the guide, the Zapier page and the changelog.
That was carried over from the limitation of polling a list, where a
deleted row stops being returned and nothing marks the moment it went. It
was never true of the activity endpoint: file.deleted is recorded like
any other action, so ?action[]=file.deleted works today.
Checked with a test rather than the enum, which turned up the shape a
caller needs: a deletion entry has no subject, because the row is gone by
the time the entry is written, so the name is snapshotted into
context.name instead. Reading subject.name there gets you null.
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.
Two things the guide got wrong, both of which would waste somebody's
afternoon.
It said tokens max out at a year and left it there. There is a "Never
expires" option on the token screen, and for an unattended integration
that is usually the right choice — the guide now says so, along with why
it is off by default and what to do instead if you keep an expiry.
And everything in it uses Webhooks by Zapier, which Zapier includes only
from its Professional plan up. Somebody on the free plan would follow the
whole page and then find they cannot add the step. That belongs at the
top, not in a support ticket.
Deleting a staff or client account is two writes: soft-delete the account,
then cascade or reassign the files and folders it owns. All four destroy()
paths (Users + Clients, web + API) ran them one after the other with nothing
tying them together.
If the second write throws, the account is already gone but its content is
not handled. The concrete way in is the reassign branch: validate() checks
reassign_to_id with exists(active), but apply() re-resolves it with
findOrFail() a moment later (AccountContentDeletion:108), so a target
deactivated or deleted in between throws — leaving a soft-deleted account
whose files still point at it, and a UserDeleted log for a deletion that did
not finish.
Wrap the delete()+apply() pair in a single DB::transaction() in each of the
four destroy() methods. validate() and the authorization guards stay outside
it: they are read-only and must be able to reject before anything is written.
cascadeDelete()/reassignTo() already open their own transaction, which nests
as a savepoint under this one, so the account soft-delete, its activity log,
and the content work now commit or roll back together.
Tests: a DeletedAccountContent double that reports content to handle and then
throws while handling it (tests/Helpers.php) drives one test per destroy()
endpoint asserting the account survives the failure and no UserDeleted entry
is written; each goes red against the un-wrapped controller.
BuildZipDownloadJob deferred every write to ZipArchive::close() but then
marked the row STATUS_READY regardless of the result:
- close() returns false when a source file was deleted between addFile()
and close() (a concurrent staff delete runs FileDiskCleanup at once) or
the disk filled up; the row went ready over an archive libzip never
wrote, and the download controller X-Accel-served a path that isn't there.
- An archive that ended up with no entries (every selected file removed or
its allowance spent before the queued job ran) is written as no file at
all by libzip, yet close() still returns true — again marked ready.
Check both the close() return and the added-entry count, and fail the row
(deleting any partial archive) when either says nothing was written.
The job also had no $tries/$timeout/failed(): a build of up to MAX_FILES
sources runs past the worker's default 60s timeout, and the kill skips the
catch, stranding the row as PENDING while the frontend polls forever. Give
it room, run it once, and add a failed() backstop that fails a row still
pending (leaving an already-resolved one alone).
Finally, purge leftover zips/{id}.zip* by row id: a killed build leaves a
partial archive and libzip temp file with no path recorded, so the path
field alone never cleaned them up.
complete() assembled the received parts into the one target file and
created the File row with no guard against a second complete() for the
same session running at the same time -- an Uppy retry, a double submit,
a resend after a lost connection. Two of them would interleave writes
into the session's single `assembled` file (the stored bytes then no
longer match the checksum computed from the in-memory buffers) and could
each create a File row.
Take a per-session lock around the finalisation and fail a second caller
fast; the lock's TTL releases the claim if a completion dies mid-flight,
so a genuine retry still works. The body moves to a finalise() helper so
complete() reads as auth + lock + finalise.
Four create flows redirected to the new record's edit page on success,
but store is gated by create_* while the edit page is gated by edit_*,
and PermissionChecker has no create-implies-edit rule. A role holding
create_* without edit_* would create the record -- write, activity log
and notifications all run -- and then meet a 403 on the success
redirect, with no way to tell the action worked and every reason to
submit a duplicate. Categories is reachable with plain UI clicks, since
the sidebar shows it from create_categories alone.
Keep landing on the edit page for anyone who may edit, and divert only
those who can't -- to the create form, which shares store's own gate
and is therefore reachable by exactly whoever just created the record;
the success toast shows there. The index would not do: Clients/Groups
lists are gated by manage_*, which store itself does not require.
Implying edit_* from create_* would not do either -- edit has no
own/others split here, so it would silently hand a deliberately narrow
create-only role edit (two-factor reset included) on every existing
record.
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.
The orphan scanner skips derived artifacts by path prefix, but the list
was a hard-coded ['thumbnails/', 'zips/'] that never learned about
'previews/'. ImageRendition::Preview caches under previews/ (and
previews/external/) on the local files disk, so every cached preview was
reported as an orphan: offered for import on the orphans screen, and
deleted by the purge command once past the grace period. An imported
preview also became a File row pointing at a path the rendition cache
owns -- destroyed the moment its source file was deleted or the cache
was flushed.
Derive the rendition prefixes from ImageRendition::cases() rather than
repeating them, so a future rendition can't be forgotten here the way
previews were; 'zips/' (the download-bundle job's) stays as it was.
Setting folder_id through update() is the same privileged reparent as
move() and bulkUpdate(), but only those two verified the target folder
was inside the caller's library (StaffLibraryScope::folders). update()
validated it only for existence, so a client-scoped staff member could
reparent an in-scope file into a folder shared with a client they are
not assigned to -- which File::scopeVisibleToClient then exposes to that
client, sidestepping the boundary the sharing endpoints enforce
(guardAssignable), and likewise into a public folder without
upload_public.
Apply the same scope->folders()->findOrFail() guard on both the web and
API update(), but only when folder_id actually changes, so re-saving a
file that already sits in an out-of-scope folder (reachable via a direct
client share) still works.
store() checks Setting::MaxFileSizeMb against the size the client declares
when it opens the session, and complete() re-checks the storage quota
against the real assembled byte count -- but nothing re-checked the size
limit itself. A client that declared a one-byte upload and then streamed
gigabytes of parts passed store()'s check and was never stopped, so the
configured limit (which store() applies to everyone, staff included) did
not hold for the resumable path that real uploads use.
Re-check the assembled byte count against MaxFileSizeMb in complete(),
cleaning up the assembled bytes and the session exactly as the quota
branch already does.
StaffAccounts::delete() soft-deletes the row. What happens to the files
and folders that account owns is a separate collaborator, and a caller
that stops at the first one leaves them pointing at an account that no
longer exists.
The docblock mentioned "the content-reassignment step" in passing, as
context for the return value, which is not the same as saying it is
required. Worth stating outright because the mistake hides: validate()
returns an empty array when the account owns nothing, so an account with
no files deletes perfectly through delete() alone, and keeps doing so
until somebody deletes a colleague who had actually done some work.
Found while reviewing a design that was about to call delete() on its
own.
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.
Having the endpoint table always visible with the tab strip halfway down
the page made the two prose documents look like a footnote to the table,
and it was not obvious there was anything to switch between.
One tab strip, directly under the heading, three views: the guide first
because it is what someone arriving here usually wants, then Zapier, then
the endpoint table. Nothing else changed.
The API has had everything Zapier needs since it shipped — a bearer
token, an auth-test endpoint at /me, and list endpoints that return
newest-first with a stable id, which is exactly the shape a polling
trigger wants. What was missing was anyone saying so.
This is written for somebody wiring up a Zap, not for somebody writing
code, which is why it is a second document rather than a section of the
guide. Same reason it renders in-app rather than linking to GitHub: the
installations most likely to need it are the ones least likely to have
outbound internet access.
It says out loud the two things that will otherwise be discovered the
hard way — a token expires within a year and nothing renews it, and a
deletion cannot start a Zap because polling cannot see one.
The 21 strings the Microsoft 365 and Gmail transports added in #1679.
Appended, so the diff is only what is new.
The two provider names stay in English on purpose — they are product
nouns, and will read as untranslated in the scanner the same way API and
OK do. The tenant-ID placeholder sits in a fixed-width input, so each
translation of it is kept to roughly the length of the English rather
than the length the sentence wants to be; a fuller Spanish rendering was
already truncating on screen.
Both fixes are follow-ups to the mail providers @denkfabrik-li added in #1679.
A refresh token is good for exactly one use — Microsoft and Google both
retire it as they issue the next one. Two queue workers finding the same
expired access token would therefore both spend it, and the loser gets
invalid_grant back. That is the same answer a revoked grant gives, so a
healthy connection would be marked broken, painted red on the settings
page and mailed to every admin. Refreshes now hold a per-connection lock
and whoever waits re-reads the row, which normally means finding a token
the winner already stored and not refreshing at all.
The Connect button read the provider dropdown, but the flow it starts
uses the saved provider. On an installation with both vendors registered,
switching without saving would open the wrong consent screen. The
dropdown now counts as an unsaved change like any other field, which also
gives it the right "save first" hint for free.
The conditional entries were pushed onto the end of the list after the
unconditional ones, so where an item appeared depended on whether it
needed a capability rather than on what it was about: Storage sat under
Languages, Email templates sat nowhere near Email, and Scheduler landed
between Branding and About.
Now there is one ordered list and each entry carries its own condition,
so the order survives whatever the edition and permissions turn on.
Both sides added a .gitignore rule in the same place: this branch's
exception for docs/email-oauth.md, and main's block for the local dev
TLS material. Keep both.
#1674 changed the code, the test and both install guides, and stopped
there — so the release notes would have shipped without the one fix that
closed#1672, and the person who reported it would have read them and
found nothing.
Written from the symptom rather than the cause: nobody searching the
changelog is looking for "reads TRUSTED_PROXIES at the wrong point in
the boot sequence", they are looking for why signing in behind Traefik
gave them an error. The upgrade note names config:cache, because a
proxy install that runs it puts the bug straight back.
#1680 fixed the redirect rendered from an exception and said plainly
what it did not cover: EnsureSetupIsComplete, EnsureAccountIsActive and
EnforceTwoFactor answer before HandleInertiaRequests is ever entered, so
a response they return never unwinds through Inertia's 302 to 303
upgrade either. Same 405, reached a different way — an account
deactivated while its owner was part-way through a form, or one being
made to enrol in two-factor.
The rule now lives in one place rather than four. Three copies of "if
the method is PUT, PATCH or DELETE" is how the fourth caller gets it
wrong, and WriteSafeRedirect can carry the explanation of why 303 —
which is worth more than the three lines it replaces, because nothing
about a bare setStatusCode call says what a browser does with a 302.
PUT /timezone is the route the setup test uses: it is one of only two
writes a guest can reach and the only one that middleware does not
exempt, so the case is real rather than defensive. All three new tests
were run against the unfixed middleware and fail there.
Extends the work of @denkfabrik-li, who found the gap and wrote it down.
A write that meets an expired session lands on the login page
A browser follows a 302 by replaying the request method, POST aside, so
a widget save whose session had expired replayed as PUT /login — and
/login takes GET and POST only. The 405 that came back hid the one
thing the person needed to be told, which was to sign in again.
Inertia already upgrades 302 to 303 for writes, but a redirect rendered
during exception handling never travels back through the middleware
stack, so the guest redirect after AuthenticationException was never
reached. bootstrap/app.php now does it for those.
Fixes#1673, reported by @mstewart14. Found, diagnosed and fixed by
@denkfabrik-li, who also wrote down the part this does not cover: the
direct redirects from EnsureSetupIsComplete, EnsureAccountIsActive and
EnforceTwoFactor sit outside HandleInertiaRequests and can still produce
the same 405 on a write.
React strips its own development warnings from a production build;
several of our dependencies do not. Radix emits an accessibility warning
for every dialog it considers underdescribed, and it was reaching
anyone who opened devtools on a real installation — advice aimed at
whoever builds the software, shown to whoever uses it.
console.error is deliberately left in. Something has genuinely gone
wrong when it fires, and a support conversation that opens with a real
stack trace is worth more than a tidy console. Only the advisory levels
are dropped, and only from production: npm run dev still shows
everything, which is where those warnings are useful.
The bundle goes from 12 console.log and 18 console.warn to none and two
— the survivors being a Recharts truthiness guard and Uppy's logger
object, neither of which speaks unless asked. What this does not do is
fix what Radix was complaining about: sixteen of twenty-three dialogs
have no description, which is a real accessibility gap and its own
piece of work.
External storage stops meaning S3 and nothing else. The Storage settings
screen asks which provider first, and Google Cloud Storage sits beside
the S3-compatible option for every self-hosted installation; the hosted
edition is handed a bucket by its environment instead, through a
capability declared here and behaviour that lives in cloud-modules.
Three of the bugs fixed here predate the feature and affect S3 users
today: share links and public group thumbnails both assumed the local
disk, and an upload the storage backend refused was recorded as if it
had been stored.
Verified against a live bucket, not only against tests — which is how
the last two were found.
The failure message names the disk, which reads as a credentials problem
even when the real cause is a bucket name that was never changed — the
exact confusion produced by switching an existing S3 configuration over
to Google and leaving the old bucket in the field.
Logged rather than shown, because 'throw' => false means the reason is
already gone by the time this code runs, and because the message goes to
whoever was uploading. That can be a client, and a bucket name is not
theirs to see.
Two bugs a green suite could not find, both from pointing the
application at a real Google Cloud Storage bucket.
The adapter attaches a legacy per-object ACL to every write, and a
bucket with uniform bucket-level access — which our own setup
instructions require, and which Google recommends — refuses it:
"Cannot insert legacy ACL for an object when uniform bucket-level access
is enabled". So the default configuration could not write to the
recommended bucket. The library ships
UniformBucketLevelAccessVisibility for exactly this, and nothing is
lost by never setting an ACL: every object here is private and every
read is a signed URL.
The second is worse and was never about Google. Both file disks are
configured 'throw' => false, so a refused write returns false rather
than raising, and LocalPartStore ignored the return. The upload reported
success, the File row was written, and the bytes were nowhere — the
listing showed a file whose download could never work. An expired S3
credential did the same thing. It now checks, and the controller already
turns that into a validation error rather than a 500, so the person
uploading is told.
Verified against a live bucket with a key scoped to
roles/storage.objectAdmin: the probe lists, writes land, reads
round-trip byte for byte, and a signed URL comes back 200 carrying
"Informe año.pdf" intact through both the ASCII and RFC 8187 forms of
Content-Disposition.
Eight new strings from the Google Cloud Storage work, and nothing else:
the scan reported the same eight missing everywhere, so this is a
translation pass rather than a backlog.
Each locale keeps the word for a bucket it was already using — kova in
Turkish, бакет in Russian, 存储桶 in Chinese — and its own level of
formality, Sie in German and vous in French against tú in Spanish and
Italian. "Google Cloud Storage" is a product name and stays as it is in
all sixteen, the way API and OK already do. The :field placeholder
survives verbatim, which is asserted rather than assumed.
Entries are inserted in place rather than appended, so each file shows
eight added lines and nothing else moved. Verified by re-running the
scan to zero missing, the Locale suite, and reading the settings screen
in Spanish in a browser — a file that parses is not evidence that a
sentence fits its button.
Three lines still told readers S3 was the only option, which stopped
being true and is the sort of thing somebody chooses a different product
over. The install guide's storage section now says what each backend is
for, that Test connection exists and is worth using before switching
uploads over, and — the part people actually get wrong — that choosing a
backend applies to new uploads and moves nothing that is already stored.
Cloud instances are given a bucket rather than configuring one, which is
the counterpart of StorageConfigure above it rather than a contradiction
of it: one edition points itself at storage, the other is pointed.
Only the declaration lives here. The behaviour is in the private
cloud-modules package, the same division Branding already uses, and
without that package the capability is inert and files stay on local
disk — so a self-hosted installation that somehow holds it is unchanged.
A redirect born in exception handling - the guest redirect after an
expired login, above all - never travels back through the middleware
stack, so Inertia's usual 302-to-303 upgrade cannot reach it. Browsers
follow a 302 by replaying the request method on the redirect target
(only POST is downgraded to GET), so a widget save whose session just
died replays as PUT /login and fails with a 405 that hides the real
"please sign in again" (#1673).
Repeat the upgrade in the exception pipeline: any 302 answered to a
PUT, PATCH or DELETE becomes a 303. Reads keep their 302, POST needs
nothing - browsers already downgrade it.
External storage meant S3 and nothing else, which is an odd hole for a
product whose users are as likely to be standing on Google Cloud as on
AWS — and paying to move bytes between two clouds to use this. The
Storage screen now asks which provider first, and the answer decides
which fields it shows, which it validates, and which driver the
files_external disk resolves to.
One disk, not two. files.disk is a stored column, so a third disk name
would fragment the data model and make every $file->disk consumer know
three names instead of two; the driver is swapped instead. A service
account key gets its own encrypted column rather than sharing `secret`,
because the two are validated, labelled and displayed differently and
one column meaning two things is how that goes wrong later.
Three things do not work by simply adding the adapter, and all three
fail quietly:
Laravel's temporaryUrl() looks for getTemporaryUrl() on the adapter,
while League's GCS adapter names it temporaryUrl(), so without the
registered callback every download and preview is a 500.
The two SDKs spell the signing options differently, and an unrecognised
one is dropped in silence — the symptom is a download named after the
storage key, not an exception. GoogleCloudStorageDriver translates, so
callers keep speaking one vocabulary, and the test asserts on the URL's
contents rather than on "a redirect happened", which is what would let
it regress.
That callback is also re-bound to the FilesystemAdapter before it runs,
so the translation is captured before registering rather than called as
$this->
`provider` is validated with 'sometimes', not 'required': absent means
S3, which is what every payload written before this choice meant, and
stops a browser holding a stale bundle from failing to save on a field
it cannot see.
Verified in a browser as well as in tests — which is how the null
provider on an unmigrated row was found, since the suite migrates and
never sees that state.
Two routes still assumed every file sits on local disk, which stopped
being true the moment external storage was switched on. A share link
answered with X-Accel-Redirect whatever the file's disk said, pointing
nginx at a path it has nothing behind; a public listing built a
thumbnail from Storage::disk('files')->path(), which for an externally
stored file is a path nobody ever wrote. Both fail only for installs
using S3, and only on those two routes, so the same file downloading
correctly from the file manager made the share link look like the
broken thing rather than where the file lives.
Neither is a new rule. FileDownloadController and
FileThumbnailController already did it right, which is the actual
finding: the knowledge was sitting in a private method on one class and
inline in another, so the next caller could not inherit it and did not.
Both are now objects with one job.
StoredFileResponse replaces InlineFileResponse and grows an
attachment() alongside inline(), since the two differ only by
disposition. LocalSourceFile takes a closure rather than returning a
path: the version that returned one also left the caller to unlink it,
and both of those are exactly the mistakes made here.
The regression tests fail against the previous controllers — checked in
both directions rather than assumed.
It is used by both OAuth mail test files, which --parallel runs in
separate processes — exactly the situation tests/Helpers.php exists
for, as its own header explains. CI caught what a whole-suite serial
run hides.
Discussion feedback: in ProjectSend "sending" can just as well mean
files. Outgoing email is unambiguous and the established name for this
screen elsewhere (GitLab, Jira, Moodle all call it that).
The tests job spent 191 of its 270 seconds running the suite one process
at a time; --parallel runs the same 1763 tests across the runner's cores
with nothing skipped. paratest is already a dev dependency.
The linter job was worse: 150 of its 195 seconds went to `pint` with no
--test and its auto-commit step commented out, so it reformatted the
runner's checkout, exited 0 and threw the result away. `npm run format`
is `prettier --write` and did the same. Both are gone, with a note on
what reinstating them as real gates would take -- a formatting sweep
first, then the flag. What remains is eslint, now read-only so it can
actually fail, and the job no longer needs PHP at all.
Both workflows now cancel superseded runs, and neither runs for a change
that only touches prose nobody's code reads. CHANGELOG.md and docs/ are
deliberately absent from that list: ReleaseNotes parses one and two
controllers serve the other.
Every page wraps itself in AppLayout, so a flashed redirect that lands on
a different page component tears the layout down and builds it again --
Toaster with it. The fresh Toaster then reads the flash at mount *and*
catches the router success event for the same visit, and every "Client
created." arrived twice. Saves that stay on the same component never
remount, which is why this survived unnoticed.
Deduping on the flash object's identity rather than its text is what
keeps the success listener doing its job: two genuine identical messages
in a row are separate objects and still both toast.
Verified in a real browser rather than by types: create a client, two
toasts before, one after, and two consecutive creates over SPA
navigation still toast once each.
Reported and diagnosed by @denkfabrik-li in #1675.
The Connect button on Settings → Connected accounts did nothing at all,
which is the kind of thing somebody upgrading needs to see written down.
Found and fixed by @denkfabrik-li in #1676.
A step-by-step guide (docs/email-oauth.md, same shape as the API
guide, whitelisted alongside it) through both vendor consoles: the
Entra app registration with its three account-type choices and what
each means for the tenant field, and the Google Cloud client with its
consent screen, test users and the testing-status 7-day refresh-token
expiry. The troubleshooting entries are errors actually hit while
building this — including Graph's ErrorQuotaExceeded, whose message
text hides that the mailbox may simply be full.
The Sending tab links to the guide right where an admin picks an
OAuth provider, via the shared links.source origin.
Same delegated shape as the Microsoft 365 provider, through the same
broker interface: the admin registers an OAuth client in Google Cloud
Console, connects the Google account the installation should send as,
and outgoing email goes through the Gmail API's messages.send as that
account.
The shared authorization-code machinery (exchange, refresh, token
storage, id_token account detection, RFC 6749 failure telling a dead
grant from a transient one) moves into an abstract OAuthCodeFlowBroker;
the two vendor brokers keep only their endpoints, scopes and consent
URL parameters. Google's quirks live where they belong: offline access
with a forced consent screen (the only way Google issues a refresh
token), and a refresh response that never re-sends one — the store
keeps what it has.
The settings screen needed no changes: the dropdown, the credential
form and the connect flow all derive from the provider enum.
Adds "Microsoft 365 (OAuth)" to the Email settings provider dropdown.
Selecting it swaps the SMTP form for an app registration (client id,
secret, optional tenant) and a "Connect mailbox" flow: the admin signs
into the mailbox the installation should send as, and outgoing email
goes through Graph sendMail as that mailbox — no password, no app
password, no SMTP AUTH, which Microsoft is winding down.
Delegated flow on purpose: it needs no admin consent and works for
work/school and personal accounts alike. Its one weakness — a grant
can die silently behind a password reset or a Conditional Access
change — is answered by a daily scheduled refresh that keeps the
token alive and, on a dead grant, warns the settings admins once
in-app and on the settings page instead of letting mail stop quietly.
Tokens and the client secret live encrypted in their own row and are
read fresh at send time, never through the boot-config cache. The
stored SMTP transport survives a provider switch untouched.
DOCKER.md said getting TRUSTED_PROXIES wrong gives you wrong client IPs
or wrong links but never affects whether a request succeeds. It does: it
is what makes the create-your-admin form come back as a 419, which is the
first thing a new install behind a proxy hits and gives no hint about the
cause. Said so, and kept the point that a 502 is a different problem.
INSTALL.md told operators never to run config:cache, and the only reason
it gave was that doing so disabled TRUSTED_PROXIES. That read now goes
through the config layer, so the reason is gone and the section claimed
something untrue. Replaced with the caveat that does apply to a cached
config: re-run it after editing .env.
Also a troubleshooting entry under the symptom people search for — 419 on
login, or being returned to the login screen at random — since the
existing proxy entry only covered rate limiting and the download log.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
publicListingFile() and publicListingImageFile() were defined in
PublicGroupsTest.php and used from PublicFilePreviewTest.php too. Pest
declares a test file's functions as ordinary globals, so that works only
once the defining file has been loaded — which under --parallel depends
on how the runner happens to distribute files across processes. Adding
any unrelated test file anywhere in the suite reshuffles that and takes
PublicFilePreviewTest.php down with "Call to undefined function", and
running it on its own with --filter never worked at all.
tests/Helpers.php exists for exactly this and its docblock describes this
failure; these two had just been missed. publicPageProps() stays where it
is, since only one file uses it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The value was read with env() inside the withMiddleware closure in
bootstrap/app.php. That closure runs when the HTTP kernel is resolved,
which is before the dotenv bootstrapper reads .env — so on every web
request env() returned null for anything set in .env, and the proxy was
never trusted. It worked when the value came from a real environment
variable, which is why the Docker compose path was fine and the manual
install described in INSTALL.md, where we tell people to put it in .env,
was not. Artisan bootstraps in the other order, so a check from the
command line reported the setting as working the whole time.
Behind a TLS-terminating proxy the consequence is not subtle. Laravel
falls back to the connecting address and the plain scheme, builds every
link and redirect with http:// while the browser is on https://, and
marks the session cookie non-secure. The browser then declines to send
that cookie to what it reads as a different, less secure origin, the
session arrives empty, and the first write fails with a 419 that reads as
"your session expired" — most often on the create-your-admin form, which
is the first thing a new install submits. Afterwards each redirect leaves
and re-enters over the wrong scheme, which is the random bounce back to
the login screen people report as flakiness.
Moved to config/trustedproxy.php, the key the framework's TrustProxies
middleware already falls back to on its own. Config files load after
dotenv, so the value is there whether it comes from .env or from the
environment.
This was also the only env() read outside config/, which means
config:cache is no longer dangerous on this application.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A download is not served by PHP. PHP authorizes it and hands the web
server the path with X-Accel-Redirect, so the web server has to open a
file PHP wrote. Where those are different users — cPanel and Plesk
commonly arrange it that way — it cannot: uploads land 0600 inside a 0700
directory, and traversing 0700 means being its owner. Nothing else on the
site shows a symptom. Uploading works, the library lists everything, and
only downloads fail, as ERR_INVALID_RESPONSE in the browser and
`open() ... failed (13: Permission denied)` in the web server's log.
FILES_WEB_SERVER_READABLE writes uploads 0644/0755 instead. Opt-in and
spread into the disk configuration rather than switched by a ternary, so
an install that does not set it keeps byte-for-byte the configuration it
had: the relaxed modes are readable by every account on the machine,
which is the wrong trade wherever the web server and PHP are one user, as
in the image and on most self-administered servers.
The two halves are not enforced alike, which is the part worth knowing.
`visibility` has Flysystem chmod each file after writing it, so 0644
holds under any umask. A directory is created by mkdir(), which masks its
mode argument, so 0755 is a ceiling: a pool at umask 0077 still produces
0700 and still cannot be traversed. That cannot be fixed from config, so
INSTALL.md carries it — how to tell the two users apart, the one-time
chmod for files already on disk, and the pool setting for the umask.
FilePermissionsTest asserts all three modes, umask cases included, since
the asymmetry is invisible from the configuration.
Reported by @denkfabrik-li (#1668), who diagnosed it and verified the
remedy on the affected host.
Three findings from one report of intermittent 502s behind Nginx Proxy
Manager, all of them ours.
Stop sending the Link: preload header. AddLinkHeadersForPreloadedAssets
copied every Vite preload into a response header, duplicating tags the
document already carried in its head — twenty on the login page. nginx
buffers a response's headers into a single block defaulting to 4 KB, so
/files, at 6060 bytes of headers, was refused with "upstream sent too big
header" and the proxy answered 502. Which pages went over depended on how
many assets they loaded, which is why it read as intermittent rather than
as a header that is always too big: the login screen fitted, the
application did not. Removing it takes /files to 1247 bytes and
/dashboard from 4544 to 1247. Nothing is lost — the browser reads the
tags in the document, and we send no 103 Early Hints.
Send nginx's logs to the container's streams. supervisord captures what
each program writes to its own stdout, but nginx opens the files named in
the package's nginx.conf as soon as it reads its config, so access and
error logs went to /var/log/nginx/ inside the container. That is where
the reason for every 502 and every 403 was written, and docker logs never
showed it — so a proxy problem presented as no logs on either side, which
is exactly how it was reported.
Document the thing neither guide covered. DOCKER.md had no reverse-proxy
section at all: no mention of proxies, of 502s, or of TRUSTED_PROXIES,
which until now was explained only in a comment in the compose example.
It gains one, including that TRUSTED_PROXIES cannot cause a 502 and is
the wrong place to dig. INSTALL.md's nginx-in-front-of-Apache path gains
the proxy_* buffer settings its fastcgi_* equivalents already had.
Reported by @denkfabrik-li (#1664), who traced it to the middleware
independently, and separately by a user running Nginx Proxy Manager who
found the too-big-header line in the proxy's own log.
The two things staff most often want to know about a file — who
downloaded it, who looked at it — were answerable only by reading the
whole activity log past everything else that had happened to it, or by
going back to the library list for the details panel.
The file's own page now has a Downloads & previews tab: the twenty most
recent times it was taken or looked at, each with who did it and the
address it went to, over a running count of both. Below them, two
buttons open the file's full history already filtered — one to every
download, one to every preview — so the narrow question is one click
and the whole log is still one click further.
Which filter value stands for "every download" is decided server-side
and travels with the payload, because it is a fact about the log's
vocabulary: downloads are three actions and share a group, previews are
one action and are filtered by name. The history page now also keeps
whatever filter it was sent with visible in its dropdown even at a
count of zero, so a button cannot land somebody on an empty table above
a select that has gone blank.
Step 2 of the v1 migration guide said Direct hardlinks your files instead of
copying them. It does not: copy is the default in both the command and the
screen, and hardlink is one of the four strategies you choose in step 3a. Say
that where the choice is first mentioned.
DOCKER.md's "Move the data you already have" reads like it is about the data in
a Legacy install. It is about relocating an already-running install's named
volumes onto the host paths chosen a step earlier, which is why it opens by
telling a new installation to skip it. Retitle it and spell out that a new
install waiting for a v1 migration skips it too — that data arrives later,
through the migration tool, and the install has to be empty when it does.
ProjectSend prints the update instructions for the way this server was
installed, and it knew two answers where it needed three: anything inside
a container was handed `docker compose pull && docker compose up -d`. On
the Compose stack that builds from a checkout there is no image behind
those containers, so `pull` skips every ProjectSend service and `up -d`
then finds them all current — the update reports success, changes
nothing, and the dashboard goes on offering the same release. Reported by
@mueller7382, who stayed on 2.0.0 that way while 2.1.0 was out (#1661).
Those installations are now their own kind, told to `git pull` and
rebuild, with the two steps a checkout needs that an image does not: its
dependencies and its compiled frontend live outside git, so a release
that moved either leaves them stale.
Two signals decide it, in that order. The published image now declares
itself with PROJECTSEND_IMAGE, which is the only evidence an operator
bind-mounting over /var/www/html can neither hide nor forge; failing that
— images published before this — a working tree in the install directory,
which the image never has and the repository's own stack always does.
getenv() rather than env(), because a cached configuration makes env()
outside a config file return null, and the answer would flip silently on
exactly the installs most likely to have cached it.
The stale-code banner keeps treating both container kinds alike: what
clears it is recreating the container, whichever way its image was built.
The changelog also credits the reporter of #1663, which was missed when
that entry was written.
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 grouped filter had a second member, "All previews", built on a
public-preview action that does not exist: previewing is recorded one
way today, so its own option already answers "who previewed this?" in
full. Static analysis caught the reference; the group would have been
unreachable even if it had compiled, since a group with a single
present member is deliberately not offered.
Previews get a group here the day a second way to preview a file is
recorded separately, and the test now pins the single-member case on a
file whose log holds one flavour of download.
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.
A file's history was only reachable from the library list, through the
details panel's Activity tab — so anyone who arrived at the file from a
link, a search or a notification had to go back and find the row they
came from to ask what had happened to it.
The file's own page now carries an Activity tab of its own, next to
General and Sharing: the twenty most recent entries, fetched only if the
tab is opened, and a link to the full history. It is behind the same
view_actions_log permission as everywhere else.
That full history is now filterable, which is the point of sending
somebody to it. The action list is built from the file's own log rather
than from the eighty-odd actions the software can record — all but a
handful of which can never apply to a file — and each option carries its
count. Downloads are three separate actions on purpose (a signed-in
recipient, a public link, the public group listing), so "All downloads"
asks that question once instead of three times; the group only appears
when the file's log actually holds more than one of its members.
Narrowing by who acted and by date range works the same as it does on
the main activity log, the reader's own calendar day included.
Deciding which update instructions to print starts with asking whether we
are running in a container, and that question is asked by looking for the
file a container runtime leaves in the root of the filesystem. Shared
hosting confines PHP to the webspace with open_basedir, where looking
outside it is a warning rather than a false — and the framework's error
handler turns warnings into exceptions, so the probe threw instead of
answering. The dashboard is the one page that asks, so it returned a 500
while everything else worked (#1663).
Suppress both probes. A host that keeps PHP inside a single directory is
not our published image, so false is the right answer as well as the
surviving one, and it lands on the manual instructions that shared
hosting wants anyway. Checking ini_get('open_basedir') instead would get
a hardened container wrong in the other direction, handing the manual
sequence to someone whose files are inside an image.
The dashboard was only the first symptom. updateNotice() reaches the same
call on every Inertia response once a newer release exists, and
RunningCodeState reaches it whenever the applied and running versions
disagree — so the next release, or the host's next update attempt, would
have taken every page rather than one.
The example compose file shipped with ADMIN_NAME/ADMIN_EMAIL/ADMIN_PASSWORD
filled in, so the entrypoint created the first administrator and nobody ever
reached the setup screen the README, the Docker Hub page and the website all
promise. Someone who followed the instructions literally — edit APP_URL and
the passwords — also ended up with a publicly reachable administrator on
admin@example.com with a password printed in a public file.
Comment the three variables out. Unattended provisioning still works for
anyone who wants it, it is just opt-in now, and the first thing a new install
shows is the setup screen again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#1658 reports that app, web, db and redis have no restart policy, so the
stack does not come back after a reboot. True, and fixed here — but the
file it is about is the development stack, and the production example has
had the policy all along. The reporter got there by following DOCKER.md,
which is #1627 again: 745f24c fixed the README's pointer and left this
page's body describing a stack no user should be running.
Against an image install almost every procedure on it was wrong. It said
uploads live in `storage/app/files/` "in the project directory" and `.env`
beside it — both are on the storage volume, and the entrypoint generates
that `.env` itself. Its compose.override.yaml recipe bind-mounted into
app, web, worker and scheduler, which are one container under supervisord
in the image, at a path one level too deep to carry APP_KEY. It told
people to chown a directory the entrypoint already chowns, to rsync from a
host path that does not exist, and to `git pull` to upgrade. Its mysqldump
read ${DB_ROOT_PASSWORD} from a .env an image install does not have, so
the documented backup silently fell back to `root` and failed. Docker Hub
links this page as "where your data lives, backups, moving to another
server".
So it is now about the image, and shorter for it: two volumes instead of
three loose things, the key explained where people actually lose it, no
override file because the compose file is the operator's own, and a
reboot section — the answer to the issue for anyone who wrote their own
compose. The clone-and-build stack keeps one pointer to CONTRIBUTING.md,
which has been the correct place for it since #1627.
The Docker Hub page keeps the two facts a reader who never leaves it
needs and hands off the procedures, so the drift that caused this has one
copy to go wrong instead of two.
Adminer and mailpit stay without a restart policy on purpose: those come
up for a session, not for the life of the machine.
Refs #1658
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The page described the application in three paragraphs and then went
straight to a compose file. Somebody deciding whether to pull it had no
idea what it looks like — and for a thing whose whole job is a screen your
clients use, that is the question they are actually asking.
The dashboard, after the paragraphs that say what this is and before the
quick start, which is the point in the page where a reader has decided they
are interested and not yet decided to spend ten minutes.
The same image the README uses, and the same alt text, which was written to
describe the screen rather than to name the file. One screenshot, not
three: the README has the other two and the caption says so, and a registry
description that scrolls past its own install instructions has stopped
being an install page.
Absolute raw.githubusercontent URL, because a repository-relative path
resolves to nothing on hub.docker.com — the same reason the badges point
there. .github/ is stripped from the release artifact, which does not
matter here: this file is pasted into a description, not shipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The page opened on a bare H1, which on a registry listing reads as an
unfinished description rather than a product. Docker Hub's own search
results, and every well-kept image beside ours, lead with a mark and a row
of badges — and the badges are not decoration there: version, size and
where to get help are the questions somebody has before they decide to
pull.
Six of them, each answering one of those: the current release, pull count,
compressed image size, stars, Discord, and the licence. Four are live
values rather than static text, so the page stops being something anybody
has to remember to update — the release badge already reads v2.1.0, and
image size already reads 78.4 MiB.
Pure markdown, no HTML. Docker Hub sanitises HTML out of descriptions, so
the centred layouts people write for GitHub silently collapse there; the
badges are consecutive markdown links, which is what actually renders as a
row. Each link carries a title, so hovering says what it is for.
The mark is apple-touch-icon.png and not the wordmark or the favicon, for a
reason worth writing down: favicon.svg has a viewBox and no width, so it
has no intrinsic size and renders at whatever the container offers — which
on a wide column is enormous. The PNG is 180x180 and renders as a mark.
That also matches README.md, which puts a small icon above the title
rather than a banner.
Colours are README.md's, not the ones on the page this was modelled after:
3b5bdb for the project, 0b7285 for the Docker facts, and Discord's own
brand colour where the badge is a Discord badge. The point is that the two
front doors look like the same project.
Every URL checked: twelve, all 200, and the four dynamic badges verified to
render real values rather than shields.io's "invalid".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was no security policy in this repository, and no "Report a
vulnerability" button either — private reporting was switched off, so the
Security tab offered an outside researcher nothing to press. The Docker Hub
description meanwhile told people to report privately through GitHub
advisories, which is advice for a thing that did not exist. The likely
outcome of that combination is somebody filing a working exploit as a
public issue, having tried to do the right thing first.
Reporting is on now, for this repository and for both public packages. This
is the policy that goes with it.
v1's policy was the starting point and about half of it survived. The
address is the same, contact@projectsend.org, which is still live and still
in CONTRIBUTING.md and both CLAs. "Do not open a public issue" is still the
point. What did not carry: it predates private reporting and would have
contradicted what we already tell Docker users, and it sent readers to v1's
SECURITY_HARDENING.md — eight kilobytes about hand-writing Nginx and Apache
rules to keep uploads private, which in v2 ship in the nginx config and are
already in place inside the image. Pointing a v2 operator at it would be
pointing them at a different architecture.
What is new is what v1 never needed: which versions are supported, now that
there are version numbers and two packages on their own lines, and a
sentence on what is not a vulnerability — an installation that skipped the
install guide, an unread scanner report, a dependency finding Dependabot
already watches.
The response times are deliberately modest and honest about the size of
this project, including what to do about silence. Better that than an SLA
nobody is on call for.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file is what Docker Hub shows above the pull command, and it has been
sitting untracked since it was written — nothing to review it against,
nothing to stop it drifting from the compose file and the entrypoint it
describes, and nothing in the repository if the machine holding it died.
Verified against what it claims rather than read for tone. The compose
block has the same services, images and environment variables as
compose.example.yaml, with nothing extra in either. supervisord really does
run nginx, php-fpm, the queue worker and the scheduler. APP_KEY really is
generated once and kept on the storage volume with .env symlinked to it.
The container really does wait for MySQL and then run
`php artisan projectsend:update`. `projectsend:admin --if-none` is what
makes "ignored once any user exists" true. nginx really does serve
protected downloads with X-Accel-Redirect, which is the reason the image
carries a web server at all. MySQL 8.0-or-newer matches INSTALL.md word for
word, and every external link resolves.
The one thing that was wrong for its audience: the Legacy section pointed
at the migration tool's repository, and everybody reading this page is on
Docker — where the tool cannot be installed the way its README implies,
because the image ships the application already built and carries no
Composer. It now points at the guide, and names the section written for
this image today.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ten task names sat in a private const as bare strings — 'Purge erased
accounts' and its nine siblings — so the Scheduler screen listed ten
English rows in the middle of an otherwise fully translated page. Found by
looking at the Spanish screen while checking the translation pass, not by
any check, because nothing could have reported it: the scan only sees
literals inside __(), and prose held as data under a key is invisible to
it. Nine of the ten had never been translatable in any language. The tenth,
'Check for updates', looked translated purely by coincidence — a button
elsewhere uses the same words, so the catalogue happened to have it.
A const cannot call __(), so the map becomes a method. That is the whole
change in substance.
The keys are untouched and stay untouched: they are the command names, they
are what the run history, the detail map, the frontend and the test
asserting this list matches the schedule all match on, and they are what
somebody types to run the thing by hand. Only the values were ever
language. The screen still prints the command name verbatim under each
translated label, which is the half a reader would copy.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The update-check work, the update-options dialog and the housekeeping
retention settings landed in English, as feature work here always does.
Twenty-eight strings, missing from every one of the sixteen catalogues,
which is a release gate rather than a nicety: build-release.sh refuses to
build while a locale is short.
They are the strings somebody reads while deciding whether to touch a
running installation — what --backup does, what happens if the dump fails,
that 0 means keep everything and that unread notifications are never
deleted whatever the setting says. Being English in a Japanese install is
worse for those than for most.
Placeholders survive verbatim, checked mechanically across all sixteen
rather than by eye: :version, :current, :date and :tag, and the literal
`sudo ./update.sh --help` and `UPDATE.md` inside the sentence that names
them.
"days" is the standalone suffix beside a number input, which is the Slavic
numeral trap — Polish, Czech and Russian inflect the noun according to the
number in front of it, and the number here is whatever the operator typed.
No single form is right for every value, so each takes the form that reads
correctly for most: dni, dní, дней.
Two collisions worth naming. "Up to date" and "Updated" are different
things — a status and a row label — and several locales would render both
the same word; they are deliberately distinct (aggiornato/ultimo
aggiornamento, up-to-date/bijgewerkt). Everything else follows the wording
already in each catalogue rather than inventing a second vocabulary for
the same ideas.
Additive only: 464 lines added, 16 removed, and every one of those 16 is a
trailing comma. Verified by scan (missing: 0 everywhere), by the Locale
suite, and by looking at the screen — the Spanish labels are the longest
of the sixteen and still fit their controls.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The guide had no answer for the install method we recommend most. The
published image carries no Composer — it ships the application already
built and has no use for one — so `composer require`, the first command in
Step 1, is not a thing a Docker reader can run at all. The guide's Docker
line assumed the stack in this repository, which builds from source and does
have one.
Composer is a PHP file, and PHP is what the image is. Fetching it into /tmp
for the length of the migration works, and the whole path is verified rather
than plausible: the tool installs, all four projectsend:migrate:* commands
register with no package:discover run by hand, and the export script the
Bundle route needs is where the guide now says it is.
Two details in there are load-bearing rather than decorative. `-u www-data`
because exec lands as root and a root-owned vendor/ is a problem the
application meets later rather than now. `--update-no-dev` because the
shipped lock file knows about pest and phpstan, and a production install
should not grow a test suite on the way past.
The section says plainly that the install lives in the container's writable
layer and does not survive the container being replaced. That is honest
about a real limit, and the reason it is acceptable is worth saying too: the
run's progress and its id map are in the database, so losing the package
costs an install, not a migration.
Step 1 is now three self-contained sections rather than one set of commands
with exceptions bolted on, because the previous shape is what let a reader
follow instructions that could not work on their install.
Verified against the real image, built locally from the release zip — the
Docker Hub repository is still private, so pulling it was not an option.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Step 1 told everybody to run `npm run build`. On a release zip there is
nothing to run it with: the zip ships its assets already compiled and
deliberately without the toolchain that compiled them, so there is no
package.json. A customer migrating a Debian install got that far, found the
file missing, and asked whether to wait for a fix. There was nothing to wait
for — they were finished and could not tell.
The larger thing that step was hiding is that `/system/migrate` does not
exist on a zip install either. A package's screens enter the frontend bundle
when that bundle is built, and app.tsx globs them at build time — for a zip
that is when the release was built, necessarily before the package was
installed. So the guide led with a screen that reader can never open, and
called the commands an alternative.
For a zip install the commands are not an alternative, they are the
interface, and they do everything the screen does. Step 1 now splits by how
ProjectSend got onto the machine, says so plainly, and points a zip reader
straight at Step 2 rather than leaving them to work out that the missing
screen is expected.
Three later steps assumed the screen too. Choosing Direct, choosing Bundle,
and downloading the export script all now read for both, and the export
script is named at its path in vendor/ — which is where it lives on every
install, and where a reader without the screen would otherwise have to guess
it was.
Not addressed here: the official Docker image ships neither Composer nor
npm, and its application directory is not a volume, so the tool cannot be
installed there at all. That is a gap in the product rather than in the
guide, and it needs its own answer.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
projectsend/community-modules is published and tagged now, so the
dependency can name a version. It could not before: with nothing to point
at, composer.json carried "*" against a vcs entry, which meant every
install resolved the default branch and got whatever had been pushed to it.
`composer validate` has been saying so on every run — "unbound version
constraints (*) should be avoided" — and it is right, because a dependency
that tracks a branch cannot be reasoned about and cannot be rolled back.
So: "^1.0", and the vcs entry goes. It has to go rather than merely stop
being needed, for the same reason the migration tool's did — a repository
declared in composer.json is canonical and outranks packagist.org, so
leaving it would keep serving dev-main off the default branch and shadow
every release ever tagged. Silently, which is the part that matters.
The lock moves with it: dev-main 311883a becomes v1.0.0, sourced from
Packagist rather than a git clone of a moving branch. Same code — the tag
was cut from that commit — but now it is a code somebody can name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
projectsend/v1-migration-tool is published now, and tagged, so
`composer require projectsend/v1-migration-tool` resolves on its own —
on any installation, new or existing, with nothing added to composer.json.
That is what the guide always claimed and what a customer's Debian install
proved it was not.
The vcs entry has to go, not just stop being necessary. Repositories
declared in composer.json are canonical and outrank packagist.org, so
leaving it would keep serving dev-main off the default branch and silently
shadow every release ever tagged. No error, no warning, just the wrong code
indefinitely — the failure mode you find months later.
So the manual fallback goes too, from the guide and from the teardown note
that told people to keep the entry. Neither describes anything that exists
any more, and instructions for a state the reader is not in are how the
last round of this went wrong: the note about a private repository outlived
the repository being private, and got skipped for looking inapplicable
while still being required.
The changelog entry is rewritten rather than added to. It described
shipping a pointer, which was the previous fix and lasted a day; what
landed is that the tool is published and versioned, and an installation can
finally say which version of it ran.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Delete a folder called Test and you could never have a folder called Test
again. The deletion worked, the folder left the screen, and the name went
with it — permanently, with an error that named a collision against a row
the interface will not show you and offered nothing to do about it.
Files and groups had it too. All three carry a unique index on slug and
all three soft-delete, so the trashed row sat in the index holding a name
nothing could reach. A public one failed outright at the validator, which
checks the table and therefore sees rows the screen does not. A private
one failed more quietly: the derived slug stepped around the trashed row
into report-2, then report-3, once per deletion, climbing forever.
The reservation was deliberate — a trashed row's slug was kept so that
restoring it could not land on somebody else's URL. But nothing in this
application restores anything. There is no restore() call, no route, no
screen; File's own comment says as much. Soft deletes are here so rows can
outlive their delete for foreign keys, the activity log and the erasure
grace period, never so they can come back. The slug was being held for a
page that could not return, and route binding already 404s the trashed row
in the meantime.
So deleting now hands the slug back, and the database is what makes that a
rewrite rather than a gentler lookup: teaching the collision checks to skip
trashed rows would leave two rows holding "report", which the unique index
rejects whatever the application thinks. The slug moves to report__deleted-42
instead. Underscores are the whole trick — Str::slug() turns them into
hyphens and Rules::slug() refuses them outright, so no derived slug and no
hand-typed one can ever land on a vacated one. That is a guarantee about
the character class rather than a hope about collisions.
The format lives in VacatedSlug rather than on the trait because the
migration needs it too and a trait constant cannot be reached through the
trait's own name — the first version of this was a fatal error waiting for
whoever ran migrations. The migration matters as much as the hook: without
it the fix only helps installations that have never deleted anything, and
every name already buried stays buried.
The collision checks still count trashed rows. It costs nothing and keeps
them honest about what the index will accept if a row is ever soft-deleted
by something that bypasses model events.
previous_file_id had this same bug and was fixed this same way, in
File::detachOnDelete — a trashed row holding its predecessor's unique slot
so the chain could never be re-linked. This is that fix, for the other four
unique indexes' worth of the same mistake. users.email is the one left, and
is deliberately not in here: an email address is a login identity rather
than a URL handle, and freeing it silently is the wrong answer.
Fixes#1645
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`composer require projectsend/v1-migration-tool` — the first command in the
migration guide, and the first thing anybody moving from Legacy types —
fails on a fresh installation with "Could not find a matching version of
package". The tool is not on Packagist, and nothing in composer.json said
where else to look, so Composer had no way to find it and said so.
The manifest already had the answer four lines up: community-modules, also
unpublished, resolves through a vcs entry. The migration tool now has the
same one, so the documented command works as documented.
The guide said to add that entry by hand, but behind "while the repository
is private" — which stopped being true when the repository went public,
and a note that reads as inapplicable gets skipped. The step it described
was still mandatory, because public and published are different things.
That paragraph is now a fallback for installations predating this commit,
which do still need it, keyed on the error text rather than on a condition
the reader cannot check.
The teardown step is inverted to match: it used to suggest removing the
repositories entry after the migration, which would now break a second
attempt. It says to leave it.
Changing repositories invalidates composer.lock's content-hash, hence the
one-line lock update.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Failed queue jobs and read notifications both grow with use, and neither
ever shrank on its own. The failed-jobs list waited for somebody to press
"Delete all failed" — a fine tool for a backlog you are looking at, and
the only thing that ever emptied it. Notifications had nothing at all: one
row per recipient per event, kept for the life of the installation, on
what is easily the fastest-growing table here.
Both now have a retention window, set together on the Scheduler screen
under Housekeeping, and a nightly purge that honours it. Thirty days for
failed jobs and ninety for read notifications, and zero means keep
everything — the explicit choice somebody makes when a failure is evidence
rather than debris.
Unread notifications are never deleted, whatever their age. A notification
nobody has looked at is the one row in that table still doing its job, and
somebody back from four months away should find their news rather than a
clean slate. The activity log is untouched by any of this: it is an audit
trail, and it is never pruned.
Two things came out of building it. The API request log purge has been
running nightly since it shipped without ever appearing on the Scheduler
screen — so a failure of it was invisible on the screen that exists to
make failures visible — and there is now a test asserting the screen's
list and the schedule are the same list, because they had already drifted
once and would again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The screen that tells somebody how to update names one command and stops,
which is right — the command is the procedure. But two of the options
behind it are the ones an administrator wants at exactly that moment:
whether it can take the backup for them, and whether they can just look
without changing anything. Both were documented only in UPDATE.md and in
--help, neither of which is open on the screen they are reading.
They are behind an "Other options" link rather than printed, so the page
in its resting state is unchanged and the one command stays the thing you
see. Four lines, the two above plus the two for a run nobody is sitting in
front of.
It opens from the dashboard card and from the update dialog both, which
means a dialog on top of a dialog in the second case. That is the right
shape here: the reader asked for a footnote to what they are already
reading, and Escape puts them back.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"Check the scheduler is running" was marked essential and hardcoded
unticked, so the getting-started list could never be completed — the two
steps that tick themselves sat above one that never would, which reads as
a checklist that has quietly stopped working.
It is answerable, and the screen it links to was already answering it: a
scheduled-run row exists once the scheduler has run on this server at all.
That is precisely what the step asks. A run that failed counts, because a
failure still proves cron reaches this installation; why it failed is the
Scheduler screen's job and the step links there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Scheduler screen printed "Check for updates · Succeeded · —" and left
it there. What it found — the whole reason that job exists — was in the
settings, which that screen never read. Somebody opening it to ask "is
there a new version?" got the answer to "did the job run?"
The Message column now carries "Up to date" or the version that is
waiting. A failure's own message still wins: what the last successful run
found is not the answer to why this one broke.
Joined at render time rather than recorded by the command, because
Laravel's scheduler fires its finished event after the command returns and
overwrites whatever the command wrote — which is exactly why that column
was empty in the first place. Reading the settings instead also keeps the
line true when the new Check now button did the work rather than the
nightly run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The update command has been recording the version it applied and the
moment it did since it shipped, and exactly one thing read it: the notice
that appears when the running code and the applied version disagree. So
the fact was written down and then only ever mentioned when something was
wrong.
About now answers the ordinary version of that question — "Updated to
2.1.0 on 17 Aug 2026" — beside the version it already showed. It is the
answer to "when did this change?", asked after something looks different,
and by whoever inherited a server from the person who set it up.
Absent rather than approximated on an installation that has never been
updated through the command: a fresh install has no update to date, and
"unknown" is noise. Same gate as the rest of that block, so a managed
installation — where the version is not the reader's concern — is
unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The check ran daily and there was no other way to run it. An administrator
who has just read that a release fixes the thing bothering them had to
reach a terminal — or wait until tomorrow to be told what the project
announced this morning.
There is now a Check now button beside the setting that schedules it. It
says what came back: the version waiting, or that this installation is
already on the newest. The time of the last check sits next to it, because
the notice on the dashboard is only as good as when it was last refreshed
and nothing said when that was.
Deliberately not gated on the daily-check setting. Switching that off says
"do not have my server phone out unattended", which is not the same
sentence as "refuse to answer when I ask" — so the button works either way
and the setting keeps governing only the schedule.
The work moved out of the command into CheckForUpdates, because the part
that must not drift between the two callers is the part with consequences:
which staff get notified, and the guard that stops them being notified
again for a release they already know about. A second copy of that in a
controller would have been found wrong six months later by somebody
receiving the same notification every time a colleague pressed a button.
Two throttles, and the second is not redundant. The route's bucket is per
user; GitHub's limit is per server address, so two administrators each
within their own allowance can still exhaust the installation's. The
cooldown is installation-wide and costs no new setting — it reads the
timestamp every check already writes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four unrelated one-liners, each already written down and none of them
worth a branch of its own.
The lock was still pinned to the community package's previous commit,
which is the one before it started shipping its own sixteen catalogues.
The mechanism that carries a package's translations to the browser landed
here last week; without this bump the release would have shipped that
mechanism with nothing to carry, and the Custom Assets screen would have
stayed half-English in every language.
The stock `local` disk had `serve` left on. Nothing in this application
writes to it, so the framework's /storage route was a door with nothing
behind it — but it was still a door, and closing it costs one word.
nginx evaluated `\.php$` before `/protected-files/`, so a protected path
ending in .php would have reached the PHP handler instead of streaming
under the sandbox headers that block sets. Not reachable on a default
install — the upload allowlist refuses php and X-Accel paths are UUIDs —
but the guarantee read stronger than it was. `^~` makes it true.
And `.release-build` is now ignored by eslint, so linting after building a
zip stops walking the vendored minified JS inside it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The frontend's catalogue was read straight out of lang/{locale}.json, so
it held exactly the strings this repository owns. That was true for as
long as this repository owned every screen — but the companion packages
own several: Branding, Custom Assets, the whole v1 import. Their strings
have been rendering in English in all sixteen languages, in silence,
because a package catalogue registered through loadJsonTranslationsFrom()
never got as far as the browser.
Asked of the framework's own loader now, which is where that registration
already lands. Same answer as before for this installation — no package
registers a path today, and the merged result is byte-identical to the
file — and the right answer the moment one does.
Precedence comes free and is the useful way round: the loader merges the
application's own catalogue last, so an installation can override a
package's wording without editing the package. There is a test for that,
because it is the kind of ordering that gets reversed by accident.
One thing the test needed and is worth knowing: SetLocale honours an
account's chosen language only while that language is enabled for the
installation, and the Settings cache outlives RefreshDatabase. A test that
sets users.locale and assumes it takes effect gets English and a very
confusing failure.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing was missing this pass — the new strings were translated as they
landed — so this is step 3, the orphan review that has been deferred for
weeks with the count sitting at 277.
Of those 277, **188 are laravel-lang's own JSON keys** (HTTP status names,
"Whoops!", "Verify Email Address", the pagination labels). Upstream and
human-maintained; the skill says to leave them alone and a lang:update
would put them straight back. Identified by diffing against
vendor/laravel-lang/*/locales/es/*.json rather than by eye.
Another **60 are strings the scanner cannot see**: prose held as data and
rendered through t(VAR) — the dashboard widget labels, the portal's sort
options, EmailTemplateSlot's descriptions, CommentingRules' refusals,
convert.tsx's plural pairs — plus three that belong to the private
packages, which the scanner does not read at all.
That leaves **29 that are genuinely ours and genuinely gone**, and this
removes them. Each was checked as a quoted literal across app/, routes/,
database/, resources/ and all three package repos, then re-checked against
every data-driven translation source. The three that looked alive
("Assigned", "Result", "System settings") turned out to be substrings of
longer live strings — "Assigned the file :subject to :target", the
LdapProbeResult class name, "System settings were updated".
Two were reworded rather than removed, which is the case that matters:
the comment-visibility description and the public-listing paragraph both
say something slightly different in the code today, so their old
translations had silently stopped applying in all sixteen languages while
the new ones are already in place.
Orphans: 277 -> 248, and every one of the 248 is now accounted for.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two things on the About screen were written when there was only one
edition. "Website" pointed at projectsend.org for everybody, and the
donation link was offered to hosted customers who are already paying for
this — on the same screen that thanks them for choosing it.
projectsend.org is the way in for the software you run yourself and
projectsend.cloud is the way in for the hosted service, so `links.website`
now resolves to whichever one the reader is actually using. That reaches
further than About by design: the "Powered by ProjectSend" line at the
foot of every outgoing email and on every client-facing page is where a
recipient meets this product for the first time, and sending a hosted
customer's recipients to self-hosting instructions is the wrong door.
The donation link is *omitted* rather than hidden by the page, so a
surface added later cannot ask a paying customer for money by forgetting
to check. Its TypeScript type is optional now, which makes the compiler
enforce the same thing.
Also fixed on the way past: the settings footer hardcoded the text
"projectsend.org" next to that link, so on the hosted service it named a
site it did not link to. It reads the host off the resolved URL now.
Verified in a browser against both editions, not only in tests. Cloud:
projectsend.cloud, no donation link, on both screens. Community:
projectsend.org and Open Collective, exactly as before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nobody on a managed installation installed ProjectSend, and the version
they are running is not theirs to keep or to change — it is looked after
for them. "Thank you for installing ProjectSend. You are running version
2.0.0, and it is yours to keep" thanked them for somebody else's work, on
the one screen whose entire job is to sound like a person wrote it.
Cloud now reads "Thank you for choosing ProjectSend" over "Your site is
ready, and keeping it running is our job." The version comes out of the
sentence there: it is true, and it is none of their concern, which is
exactly what makes it noise.
Community is unchanged, and so is the wording somebody sees revisiting
the page later — that one is neutral already.
Branched on the shared `edition` prop rather than a capability, because
the question being asked really is which edition this is: SystemUpdates
happens to line up today, but it is about who may update, not about who
installed.
Both new strings are in all sixteen catalogues in the same commit, since
this is a two-string change and splitting it would leave the hosted
greeting English-only for however long the next pass takes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Twenty-four strings each: the thank-yous, the eight quick-start steps and
their one-line descriptions, the Essential label, and the line from the
website at the foot of both pages. 336 entries in, 48 out.
The three removed are the what's-new hero sentences reworded earlier
today — a translation that silently stopped applying the moment the
English changed, which is exactly the orphan the skill says to clear. All
three were checked against resources/ and app/ first; none survives as a
literal anywhere.
Register follows each catalogue rather than the language's default, and
terminology follows the screen each step links to: the scheduler step
says "programador de tareas" in Spanish because that is what the sidebar
calls it, "Planer" in German, "Harmonogram" in Polish.
"Made with care in Argentina" is translated rather than left in English —
it is a sentence about the people who made this, and the people reading
it in Japanese deserve to read it too.
Checked in the browser with the interface in Spanish: "Gracias por
instalar ProjectSend", "Estás usando la versión 2.0.0, y es tuya",
IMPRESCINDIBLE on the mail server and the scheduler, and "Hecho con
cariño en Argentina. Compartido con el mundo." under the fold line. Scan
reports 0 missing in all sixteen; the Locale tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"Made with care in Argentina. Shared with the world." — projectsend.org's
own line, at the foot of the two screens where somebody is being greeted
rather than getting on with their work.
Drawn rather than set as the flag emoji the website uses. Regional
indicator pairs have no glyphs on Windows or on most Linux desktops, and
both fall back to a pair of small letters — the first render of this
component read "AR Made with care in Argentina" on the machine that
screenshotted it, which is what a good half of the people self-hosting
this would have seen. Eleven lines of SVG look the same everywhere.
The Sun of May is a plain disc: at sixteen pixels its rays are a smudge,
and a smudge reads as a rendering fault rather than as a flag.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both greeting pages read like status reports. The install page opened
":name is installed and yours" without saying which version, the update
page opened "The update finished, and everything came back up", and the
quick-start list was eight full-width rows of two-line descriptions —
about 1400px, with the last two steps and the invitation below the fold
on a laptop.
The install page now thanks somebody for installing ProjectSend and names
the version they are on. The update page thanks them for updating and for
continuing to trust it with their file sharing. Both revert to plain
wording when the page is opened later from a link: thanking a reader
again for something they did months ago is the cold thing, not the warm
one.
The list becomes a two-column grid of icon cards — four rows instead of
eight, 1000px against 1490px, which is one screen. Icons come from the
sidebar's own vocabulary, so the chip on a card is the icon on the screen
it opens. Descriptions are one short clause each; the screen at the other
end explains itself.
And the steps stop pretending to be equally urgent. QuickStart now says
which are essential — the two that make this application do anything at
all, the mail server, the scheduler — and those carry an amber chip and
a label, against the brand colour for everything else and green for the
done ones. Amber is not invented here: it is the warning Alert variant's
palette, reused verbatim so dark mode is somebody else's solved problem.
The Discord card was two identical copies within an hour of each other,
so it is one component now, before the pair could drift.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Setup ended by handing somebody a login form and an empty dashboard.
Everything this application can do was one menu away, and which menu was
theirs to discover.
The first time the administrator signs in to a new installation they now
land on a short ordered list of what is worth doing first — add a client,
upload a file, group the people who get the same things, choose how the
file lists and the email look, point it at a mail server, add the team,
check the scheduler — each a link straight to the screen that does it.
The list is filtered twice, and both filters matter. By permission,
because a link that answers 403 is worse than no link. And by edition:
a managed installation is not sent off to configure a mail server
somebody else runs, to create staff accounts that are not its to create,
or to check a scheduler it does not host. Those three drop out on Cloud
and the other five remain.
Two steps tick themselves, because the database can answer them: a client
exists, a file exists. Nothing else is checkable without guessing — a
theme that was never changed looks exactly like one chosen deliberately —
and a tick meaning "we assume so" is worse than no tick.
The invitation to the Discord is at the very bottom, after the list.
Somebody who has just installed this came with a job in mind, and opening
with a social invitation is the fastest way to lose them.
The marker is raised where a first administrator comes into existence —
the setup screen and `projectsend:admin`, so a container provisioned from
environment variables is welcomed too — and it is false by default, so an
installation that updates into this feature is not congratulated on an
install it finished a year ago.
RedirectToWhatsNew becomes RedirectToGreeting and answers for both: they
are the same interruption, and a second middleware on the same route
would have to know about the first to avoid arguing with it. Installing
wins; release notes for a version you never ran are the wrong greeting.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The eleven strings the welcome page introduced, now in every catalogue:
176 entries across sixteen files, appended rather than merged in, so the
diff is only what was added and no existing line moves.
Two of them are deliberately identical to the English. "Discord" is a
product name, and "ProjectSend :version" is a name and a placeholder with
nothing between them to translate — the scan counts both as untranslated
and that is the right answer.
Register follows each catalogue rather than the language's default: Sie
in German, vous in French, je in Dutch, Ty in Polish, tú in Spanish. The
thank-you sentence was written to sound like somebody meant it, since a
literal translation of a warm line is a worse translation than a free one.
Checked in the browser with the interface in Spanish, not only scanned:
"ProjectSend ahora está en la versión 2.0.0", "Ven a saludarnos", "Únete
a Discord", "Novedades" — every line fitting its space. Scan reports 0
missing in all sixteen; the Locale tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
On `muted` it was a grey box between two things people actually look at.
It now sits on `accent` — the brand colour at surface strength, which
already carries a dark-mode counterpart, so this is legible in both
without a hardcoded purple anywhere and follows the palette on an
installation whose branding replaces ours.
Its button is filled and the one below it is not. Outline was wrong here
twice over: its hover state is this exact background, so the button
vanished under the cursor, and "continue to the dashboard" is leaving —
not the thing this page is encouraging.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An update finished and nothing said so. The dashboard looked identical to
yesterday's, and whatever the release brought was in a file nobody opens.
The first time the installation's administrator opens ProjectSend after
an update, they now land on a page that names the version they are on,
invites them to the Discord — the same invitation update.sh prints, made
again where they are actually looking — and then lays out what the
release brought.
The notes come from CHANGELOG.md inside the release, not from GitHub: the
one moment this page exists for is the moment after an update, possibly
on a server with no outbound access, describing code already on disk.
Parsed rather than rendered, so nothing in it can become HTML.
Once, and to one person. The update happened to the installation, so
greeting five staff members — each having to dismiss a page they did not
ask for — would turn a pleasant moment into a support question. It goes
to the oldest active administrator, which on any installation that went
through setup is whoever set it up. No owner flag was invented for this:
administrators are equal in authority, and changing that for a greeting
is not a trade worth making.
Only forwards, and only for a real update. A fresh install has nothing to
catch up on, a container reboot has not updated anything, and somebody
restoring an older release is dealing with a problem rather than
celebrating. Managed installations never see it at all — nobody signed in
there performed the update it thanks them for, which is the same gate the
System card and About's environment block already carry.
The redirect is attached to the dashboard alone, not the web group: it
catches a login and the sidebar logo both, without ever interrupting a
download to congratulate somebody. Reading the page clears the marker,
but the address keeps working — closing it by accident should not be
unrecoverable — and About now links to it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The last thing an operator sees after a successful update is now where the
people are. Printed rather than asked: the answer to "would you like to
join?" is a browser, and this runs over SSH on a server that has none, so
a y/n could only print the URL its "no" branch prints anyway — and --yes
would answer it on behalf of a cron job that cannot join anything.
Only on success. --check is a report somebody may be scripting, and a
failed run's last line should stay the recovery instructions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Somebody followed the README's Docker quickstart, which starts the
development stack, and got three failures in a row with nothing to search
for (#1627): the worker died once a second on a missing autoloader, the
site answered a bare 500, and once dependencies were installed by hand the
setup screen threw ViteManifestNotFoundException.
None of that is wrong behaviour for a clone — vendor/ and public/build/
are deliberately not in git — but every one of those failures kept its
cause to itself.
The preflight guard exists to turn "this was never set up" into a
sentence, and it runs before the autoloader precisely so it can. It now
answers two more questions: dependencies not installed, and frontend not
built. The dependency check goes first, before the .env one, because the
fix that branch prints — php artisan key:generate — cannot itself run
without the autoloader, so reporting the key first hands somebody a second
and more confusing error. A running vite dev server counts as built:
public/hot means the assets come from there, and blocking a developer
mid-session would be worse than the exception this replaces.
The worker and scheduler exec straight into artisan, so before composer
install they died instantly and restarted forever, filling the log that
had to be read to fix it. They now print what is missing and exit slowly,
and recover on their own once it is there. The scheduler gains the restart
policy the worker already had — without one it exits during that window
and stays exited, and scheduled work then silently never happens.
Rehearsed on a genuine clone of the public repository, following the
reporter's exact path: worker prints instructions instead of fatals (2
restarts in 30s, not 30), the browser gets "ProjectSend is not installed
yet" naming composer install, then "not configured yet", then "not built
yet" naming npm run build, then the setup screen.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The README told everybody to clone the repository and run the development
stack, which builds nothing and installs nothing — so a user following it
got a crash-looping worker and a 500, and reported it as a bug (#1627).
They were reading the right document; it was pointing at the wrong stack.
Getting started now says what each path is for. Users fetch
compose.example.yaml and run the published image, which ships vendored and
pre-built and needs no toolchain — it has existed all along and was linked
from nowhere in the README. Contributors get one paragraph saying a clone
is a development copy, not an installation, and a pointer to
CONTRIBUTING.md, whose sequence was already correct.
CONTRIBUTING gains the sentence its readers were missing — why those steps
exist at all — plus the two things a contributor now meets: containers
that name the missing step instead of dying silently, and the fact that a
second checkout shares one Compose project name and will take over the
first one's containers. That last one cost me this working copy's stack
for a few minutes.
Also: DOCKER.md and INSTALL.md both linked README.md#development, an
anchor that has never existed. And UPDATE.md's build-from-source path told
operators to pull and rebuild with no dependency step, which is the same
trap on every release that moves composer.lock.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The English for the updater, the stale-code banner and the activity-log
entry had settled, so this is their pass: 208 lines across sixteen
catalogues, every :placeholder intact, no existing entry reordered.
The activity row and its filter label were modelled on each catalogue's
own "Installed ProjectSend" / "ProjectSend was installed" pair, so the
new rows read like the ones beside them rather than like a different
translator wrote them.
One code change belongs to this pass rather than to the feature: the
update entry recorded "an unrecorded version" when the previous version
was unknown, and context values are substituted into the template
verbatim — never translated — so that sentence fragment would have sat
inside an otherwise Japanese or Polish row forever. It is a dash now.
Verified on a real install with the interface switched to Spanish:
"Sistema — Actualizó ProjectSend a 2.0.3, desde 2.0.2", and the action
filter reading "ProjectSend fue actualizado a una versión nueva".
Scan reports 0 missing in every locale; the Locale tests pass.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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>
Recent Debian and Ubuntu nginx packages set HTTP_HOST to $host in
fastcgi_params, on purpose — it keeps a client-supplied Host header away
from the application — and $host carries no port. On 80 and 443 nobody
notices. Serve the site on 8080 and every absolute URL the application
builds points at port 80 instead: the login redirect lands somewhere that
does not answer, which reads as ProjectSend being broken rather than as a
web server default having changed.
Found on a manual install of our own, serving on a non-default port,
where the site answered 200 and the browser still got a 404.
The official image is unaffected — Alpine's fastcgi_params has no such
override, so nginx passes the Host header with its port.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
`chown -R` on the install directory also tries to change the ownership of
public/storage, which is a symlink — and some filesystems refuse that even
for root. It turned up on a bind-mounted install (a `fakeowner` mount
refuses lchown outright) and would equally turn up on an NFS export with
root_squash. The update stopped there: files replaced, database not yet
migrated, which is the worst place to stop.
A symlink's own ownership decides nothing, since access is governed by
whatever it points at, so those are now skipped rather than tolerated by
ignoring errors that might have mattered elsewhere.
Found by running the thing on a manual install rather than by reading it.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Updating a server install cost nine artisan invocations plus a PHP-FPM
reload, written out in three places that had already drifted apart. One
of those steps is silently fatal to skip: with opcache.validate_timestamps
off — what production guides recommend and what our own image ships — the
database moves to the new version while every visitor keeps being served
the old code, and artisan reports the new version throughout.
`sudo ./update.sh` is now the whole procedure. It asks whether to check
GitHub, asks whether to download the release and verifies the checksum
published beside it, and asks whether there is a backup — offering to dump
the database when the answer is no. Then it takes the site down, replaces
the files, runs the update, reloads PHP-FPM, restarts the worker and
brings the site back. The application still has no self-updater: nothing
is fetched or applied unless somebody runs this and answers yes.
Underneath it is `php artisan projectsend:update`, which is everything an
update does that needs no root — and now the only definition of it. Both
container entrypoints call it instead of carrying their own copy of the
sequence, so the two paths cannot drift again.
Three findings worth keeping in the record, all from rehearsing rather
than reasoning:
- queue:restart has to come last. It writes its signal into the cache,
so clearing the cache afterwards deletes it and the worker runs old
code forever.
- optimize:clear is not safe to recommend. It runs cache:clear, which
on Redis is FLUSHDB — harmless on the default two-database layout,
but on a single-database Redis it takes the sessions and the queue
with it. The compiled caches are cleared individually instead.
- update.sh overwrites itself mid-run, because the zip contains it and
bash reads its own script lazily by byte offset. It re-execs from a
temporary copy before touching anything.
And when the reload is skipped anyway, the application now says so:
projectsend:update records the version it applied, and any staff page
compares that with what the running process actually compiled. The same
check catches the mirror image — new files in place, update never run.
Rehearsed end to end against real installs: a container upgrade (69 to 73
migrations, key and data intact, healthy), a scripted update on a real
nginx + php-fpm install with OPcache pinned (web process moved 2.1.0 to
2.1.1), the skipped-reload case (banner appears naming both versions, and
clears on reload), the refusals (downgrade, non-release zip, truncated
zip, URL passed to --zip, non-root), a database taken down mid-update
(site comes back out of maintenance mode by itself), and a real download
of the published 2.0.0 zip with its checksum verified.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Updating had a numbered list inside INSTALL.md and a code block in the
dashboard, and between them they were missing the step that decides
whether an update works at all.
Rehearsed both paths end to end against a real 2.0.0 install before
writing any of this down. On Docker the whole update is `docker compose
pull && docker compose up -d`: the entrypoint waits for the database,
migrates, ensures the roles, and starts the workers, and it does not
start nginx until that finishes — so a failed migration is a container
that stays down rather than a half-updated site taking traffic. The
generated APP_KEY on the storage volume is left alone. Verified: 69
migrations to 73, key unchanged, data intact, healthcheck healthy.
The manual path is where the gap was. Replace the files, run the four
documented commands, and the site still serves the old version: with
`opcache.validate_timestamps=0` — what every production guide recommends,
and what our own image ships — PHP never re-reads a file it has already
compiled. The database is on the new version, every visitor is on the
old code, and `php artisan` cheerfully reports the new version the whole
time you are trying to work out why. Reproduced exactly that, then
watched a php-fpm reload fix it.
UPDATE.md is now the whole procedure for both, INSTALL.md keeps the short
sequence with the reload added, and the in-app instructions gained the
same line — in the code block, which is not translated, so no locale is
left saying something different.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Same frame, same data and the same 1360x850 at 2x as dashboard.png,
captured with the appearance in dark. Not referenced from the README —
it is here for anywhere the dark one suits better.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The retake was 2720x2664 — near square, because it fitted both columns
of widgets end to end. Next to files.png and portal.png, both 16:10, it
read as a different kind of picture and took three times the vertical
room in the README for it.
Same 1360x850 at 2x the other two use. The point was never that every
widget be legible in one frame — only that they all be switched on, so
the installation looks like one somebody uses rather than one nobody has
configured.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The old one photographed a development database: 58 files totalling
7.3 MB, "template-business" and "new-editor" among the largest, two
clients in the storage table, and eight identical "Logged in" rows as
the recent activity. It showed the layout and none of the point.
This one is a plausible installation — real-sounding client
organisations against their quotas, files with names and sizes somebody
would recognise, a month of uploads and downloads with a working week's
rhythm in it, and activity that says what people did rather than that
they arrived. Every widget is on, in the two-column layout, so the
picture also answers "what does this thing actually track".
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The image creates a fixed-uid www-data for php-fpm but left nginx on the
alpine package's own `user nginx;` (uid 100), while nginx.conf's header
claimed the two already matched. They did not, and the claim is why
nobody looked.
It matters because php-fpm hands nginx files to serve: protected
downloads and thumbnails go out by X-Accel-Redirect. Directories the
application creates on demand come out at Flysystem's private mode, 0700,
owned by www-data — and traversing one of those means being its owner,
since there are no group or other bits to fall back on. So the first
thumbnail an installation ever renders answers 403, from nginx, with
nothing in the application's log to show for it (#1614).
Pointing the directive at www-data is half the fix. /var/lib/nginx and
its tmp/ arrive from the package owned by the old user, and nginx
recreates only the leaf temp directories at boot — as the new user, so
they look correct while their parent stays untraversable. Every request
nginx buffers to disk then fails with a bare 500 that never reaches
php-fpm, which is every upload chunk, which is every upload (#1618).
Fixing one without the other trades a broken thumbnail for a broken
upload, so both land together.
Verified by building the image three ways and driving a real upload and
thumbnail through each: unchanged, the part PUT succeeds and the
thumbnail 403s; with the user directive alone, the thumbnail works and
the part PUT is a bare nginx 500 with "open()
/var/lib/nginx/tmp/client_body/0000000001 failed (13: Permission
denied)"; with both, the chunked upload completes, the thumbnail renders,
and the file downloads back byte for byte.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The tab title suffix came from `import.meta.env.VITE_APP_NAME`, with the
starter kit's own `|| 'Laravel'` behind it. Vite resolves that at build
time, and a release ships public/build/ already compiled — precisely so
nobody downstream needs npm — so whatever the build machine had is
frozen for every install that artifact produces. The 2.0.0 zip has
`const s2="Laravel"` in app-BoPagLMd.js, and no APP_KEY, APP_NAME or
VITE_APP_NAME an operator sets afterwards can reach it (#1619).
Swapping the fallback to 'ProjectSend' would fix the reported symptom
and leave the mechanism: a name baked at build time, wrong for anyone
who renamed their site. The name is already on every page — the shared
props carry the site_name setting — so read it from there and drop the
build-time variable entirely, .env.example's dead VITE_APP_NAME line
included.
The root view's own <title> now reads the same shared prop, so the tab
does not show one name before hydration and another after on any
installation whose administrator renamed the site.
Verified in a real browser, which is the only place this is visible —
the server-rendered title was always right, and curl never saw the bug.
Titles read "Log in - ProjectSend" and, after renaming the site with no
rebuild of any kind, "Log in - Acme Files".
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The base php:*-fpm image creates /var/www/html owned by its own www-data
(uid 82) and mode 1777, so that an image can run as an arbitrary user.
This image replaces www-data with a fixed uid 1000 and copies the
release in with COPY --chown — which re-owns what it copies into the
directory, never the directory itself. It was left world-writable,
sticky, and owned by a uid the container no longer has.
fs.protected_symlinks — on by default on Ubuntu, Debian and most current
distributions — then refuses to let a non-root process follow a symlink
in such a directory, and .env is exactly that: the entrypoint keeps it
on the storage volume so a generated APP_KEY survives container
replacement, and links it into place. So every request 503'd with
"ProjectSend is not configured yet" while `docker exec ... cat .env`,
run as root, printed the file back perfectly (#1615).
Three changes, each independently sufficient for the reported case, and
deliberately so — this failure is silent and its symptom points away
from its cause:
- the image owns /var/www/html as the runtime user, at mode 755;
- the entrypoint owns the symlink it creates, so it stays followable
even if that directory's mode ever drifts back;
- preflight distinguishes "no .env" from ".env is there and cannot be
read", instead of reporting the second as the first and sending the
operator off to create a file they already have.
Verified by building the production image before and after: every
request 503s beforehand, with /var/www/html at uid 82 mode 1777 and
www-data denied on the symlink while root reads it; afterwards /up
answers 200, the container reports healthy, and / redirects to /setup.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
An empty response gets Symfony's default Content-Type, text/html, and a
CDN in front of the app takes that at its word: Cloudflare's Email
Obfuscation and Automatic HTTPS Rewrites both rewrite HTML bodies, so
they drop the origin's ETag from the response — a rewritten body would
no longer match it.
That ETag is the client's only signal that a part landed. Nothing on
this side notices its loss, because LocalPartStore keeps its own record
of every part and complete() never reads a client-supplied one; the
upload simply reaches 100% and stops, with no error at either end.
Reported from a Cloudflare-fronted install (#1616), where the visible
symptom was Uppy's "Could not read the ETag header" — which names CORS,
and sends you chasing a preflight that same-origin requests never make.
Naming the content type accurately keeps the response out of every
HTML-rewriting path there is, rather than asking each CDN-fronted
install to discover this one for itself.
The rationale sits inside the header array rather than above the return:
Scramble reads a comment attached to a return statement as that
response's description in the published OpenAPI document, and this
controller is mounted on the API routes too.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The lock pinned a commit that no longer exists: that repository's
history was replaced with a single commit, so an install resolving from
the lock would have failed to fetch it.
Nothing about the package's contents changed — same code, same version
constraint, new commit id.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-14 01:50:03 -03:00
334 changed files with 27477 additions and 1958 deletions
# This condition belongs to the job, not to the step below it, and moving
# it back down would quietly cost money. A step that is skipped has still
# had a runner allocated for it; a job that is skipped never gets one, and
# Actions bills per job that runs. `issue_comment` fires on every comment
# in the repository, so with the check one level lower every "thanks,
# merged" on a pull request — and every comment on a plain issue — spun up
# a machine to decide it had nothing to do.
#
# GitHub cannot filter `issue_comment` by body at the `on:` level, so this
# is the only place the decision can be made.
#
# `issue.pull_request` is present only when the comment is on a pull
# request; comments on ordinary issues have nothing for this action to
# check.
if:>-
github.event_name == 'pull_request_target'
|| (github.event.issue.pull_request
&& (github.event.comment.body == 'recheck'
|| github.event.comment.body == 'I have read the CLA Document and I hereby sign the CLA'))
runs-on:ubuntu-latest
steps:
- name:CLA check
if:(github.event.comment.body == 'recheck' || github.event.comment.body == 'I have read the CLA Document and I hereby sign the CLA') || github.event_name == 'pull_request_target'
projectsend-worker`. INSTALL.md has the full file, and the two-worker setup if you would rather
keep the two kinds of work apart. Docker installations need no change.
If it is ever missed, ProjectSend now says so on screen: staff who can see system information get
a banner naming the problem and the fix.
- **If you run behind a reverse proxy, check `TRUSTED_PROXIES`.** It is now read correctly, which it
was not before. Set it in `.env`, and do not run `config:cache`, which stops `.env` being read at
all.
Thanks to [@denkfabrik-li](https://github.com/denkfabrik-li), who found, diagnosed and fixed most
of the boundary work above, and to [@mstewart14](https://github.com/mstewart14),
[@elibrachas](https://github.com/elibrachas), [@mueller7382](https://github.com/mueller7382) and
[@pabloalvarez44](https://github.com/pabloalvarez44) for reports and fixes.
### Issues closed since 2.1.0
The summary above is what changed. This is the paper trail, for anyone who wants to read the
original report.
- [#1627](https://github.com/projectsend/projectsend/issues/1627) — Errors while installing via Docker
- [#1648](https://github.com/projectsend/projectsend/issues/1648) — A deleted account's email address can never be used again
- [#1661](https://github.com/projectsend/projectsend/issues/1661) — Docker update instructions do not update ProjectSend when using official Compose setup
- [#1662](https://github.com/projectsend/projectsend/issues/1662) — Preview files not available on v2.1.0
Read this before you put real files in ProjectSend, not after.
> Getting started with Docker in the first place is covered in [README](README.md#development).
> **This page is about the official image**, `projectsend/projectsend`, started from the
> `compose.example.yaml` in [Getting started](README.md#getting-started). That is the supported way
> to run it.
>
> A **clone of this repository is a development copy, not an installation** — it builds from source,
> bind-mounts the working tree, and ships nothing pre-built. If that is what you are running, its
> setup and its data layout are [CONTRIBUTING.md](CONTRIBUTING.md), not this page.
>
> Installing without Docker, on a plain PHP server, is [INSTALL.md](INSTALL.md).
---
## The three things that matter
## The two things that matter
Everything ProjectSend cannot regenerate lives in exactly three places:
Everything ProjectSend cannot regenerate lives in exactly two Docker volumes:
| What | Where it is by default | Losing it means |
|---|---|---|
| **The database** | A Docker *named volume*,`projectsend_db-data` | Everything except the files themselves: accounts, groups, permissions, share links, comments, the activity log |
| **Uploaded files** | `storage/app/files/` in the project directory | The files your clients downloaded — gone |
| **`.env`** | The project directory | `APP_KEY`, without which saved SMTP and LDAP passwords cannot be decrypted |
| **The database** | The volume mounted at `/var/lib/mysql` —`projectsend_db-data` | Everything except the files themselves: accounts, groups, permissions, share links, comments, the activity log |
| **Uploaded files, and `APP_KEY`** | The volume mounted at `/var/www/html/storage` — `projectsend_storage` | The files your clients downloaded, and the key that decrypts saved SMTP and LDAP passwords |
The second one is the one people get wrong, because it is two things in one place. The container
generates `.env` on first boot and keeps it *on the storage volume*, at `storage/.env`, symlinked
into place — precisely so `APP_KEY` survives the container being replaced. A key that changes
between restarts signs everybody out and makes every encrypted column permanently unreadable, and
nothing errors when it happens. Back up the volume and you have both halves; back up only
`storage/app/files/` and you have the files without the key.
(If you set `APP_KEY` in the environment instead, Laravel reads it from there and it wins. That is
the right move when you already manage secrets somewhere else — but then it is *that* system's
backup you are relying on.)
You do not have to work out which of these you have from memory. **The dashboard's System panel
reports where your uploaded files actually live** — a host directory, a Docker volume (named), or
@@ -36,14 +53,14 @@ Two things you may be surprised to find you do **not** need to protect:
everyone out and drops any not-yet-sent emails or half-built zips. Annoying; not data loss.
- **Parts of `storage/app/files/`** are derived, not precious: `zips/` (built downloads, deleted
automatically after a day), `thumbnails/` and `previews/` (rebuilt on demand the next time
somebody looks at a file). They sit in the same directory as the real uploads, so the simplest
thing is to back up all of it and not think about which is which.
somebody looks at a file). They sit inside the volume you are backing up anyway, so the simplest
thing is to take all of it and not think about which is which.
## The good news, and the one command to fear
Named volumes are already outside the container lifecycle. `docker compose down`,
`docker compose up --build`, deleting and recreating every container — none of those touch
`projectsend_db-data`. Upgrading does not lose your database, and never did.
Named volumes are already outside the container lifecycle. `docker compose pull`,
`docker compose down`, deleting and recreating every container — none of those touch
`projectsend_db-data` or `projectsend_storage`. Upgrading does not lose your data, and never did.
The command that *does* destroy it is:
@@ -52,73 +69,185 @@ docker compose down -v # ← the -v deletes the named volumes
```
That flag exists to clean up a development machine. On a real installation it deletes your entire
database in about a second, with no confirmation. The same goes for`docker volume prune` and
`docker system prune --volumes` when the stack happens to be down.
database and every uploaded file in about a second, with no confirmation. The same goes for
`docker volume prune` and `docker system prune --volumes` when the stack happens to be down.
So the actual problem with the default setup is not fragility, it is **invisibility**: your
database is somewhere under `/var/lib/docker/volumes/`, which means most people never back it up
and would not know where to look. The rest of this page fixes that.
So the actual problem with the default setup is not fragility, it is **invisibility**: your data is
somewhere under `/var/lib/docker/volumes/`, which means most people never back it up and would not
know where to look. The rest of this page fixes that.
---
## Surviving a reboot
Every service needs a restart policy, or the Docker daemon will not start it again when the host
comes back:
```yaml
services:
app:
restart: unless-stopped
db:
restart: unless-stopped
redis:
restart: unless-stopped
```
`compose.example.yaml` already has this on all three. It is worth checking if you wrote your own
compose file, because the failure is silent and delayed: the stack works perfectly until the first
reboot or power cut, and then the site is simply down with no error anywhere. `depends_on` does not
cover this — it applies to `docker compose up`, not to containers the daemon brings back at boot.
```sh
docker compose ps -a # after a reboot, everything should be Up, not Exited (0)
```
---
## Behind a reverse proxy
Almost nobody exposes the container directly: there is a proxy in front terminating TLS — Nginx
Proxy Manager, Traefik, Caddy, or an nginx vhost you wrote. Two things are worth setting before you
go looking for a bug that isn't there.
### Tell ProjectSend the proxy is there
```yaml
environment:
TRUSTED_PROXIES: "*"
```
Without it every visitor appears to come from the proxy. The login rate limiter then treats all of
your users as one attacker, and the download log records the proxy's address instead of the
@@ -47,12 +47,12 @@ per-seat pricing. It runs on your server, and the files stay there.
- 16 languages
- A REST API with scoped tokens and generated OpenAPI docs
- Privacy controls, including GDPR-grade account erasure with a grace period
- Local disk or S3-compatible storage
- Local disk, S3-compatible storage, or Google Cloud Storage
## Screenshots
<p align="center">
<img src=".github/screenshots/dashboard.png" alt="The dashboard, showing counters for files, clients and groups alongside largest files, recent activity and system information" width="900">
<img src=".github/screenshots/dashboard.png" alt="The dashboard: counters for files, clients and groups, the clients using the most storage against their quotas, a month of uploads and downloads as a line chart, and recent activity" width="900">
</p>
<p align="center"><em>The dashboard — what is in the installation, and what has been happening in it.</em></p>
@@ -68,17 +68,18 @@ per-seat pricing. It runs on your server, and the files stay there.
## Getting started
**With Docker** — the quickest path, and the one we recommend.
**With Docker** — the quickest path, and the one we recommend. Nothing to build: the published
image ships with its dependencies and its frontend already compiled.
It is the "Report a vulnerability" button on this repository's Security tab. The report stays
private between you and the maintainers, the whole exchange lives in one place, and it is the route
that can end in a published advisory with a CVE and your name on it.
If you would rather not use GitHub, or the report does not fit that form, email
<contact@projectsend.org> instead. Either is fine. What matters is that it does not start in
public.
**Please do not open a public issue for a security report.** An issue is world-readable the moment
it is filed, including by people running the version you just described how to break.
### What helps
Enough to reproduce it, and nothing you would not want to write down:
- The version, from **System → About** (self-hosted) or `config/projectsend.php`.
- How the installation is deployed — the Docker image, a manual install behind nginx, something
else — and anything unusual in front of it.
- The steps, and what you saw. A short recording or a `curl` command beats a description.
- What an attacker gets out of it, if it is not obvious.
You do not need a proof-of-concept exploit, and you should not run one against an installation that
is not yours.
### What to expect
An acknowledgement within a few days, and a real answer — a fix, a plan, or a reason it is not
what it looked like — once it has been reproduced. If a fix ships, you are credited by name unless
you would rather not be.
This is a small project. If a week goes by in silence, assume the message went astray rather than
that it was ignored, and send it again.
## What is in scope
Anything that lets somebody reach a file, an account, or an installation they should not: the
sharing and permission rules, authentication and two-factor, the public pages and share links, the
API, the upload and download paths, and the setup and update flows.
Some things are worth a report but are not vulnerabilities in ProjectSend:
- **An installation that has not done what the install guide says.** Serving the storage directory
straight from the web server, or running without the protected-file rules, is a deployment
problem — see [INSTALL.md](INSTALL.md) and [DOCKER.md](DOCKER.md). Tell us anyway if the
documentation is what led somebody there.
- **Findings from a scanner, unread.** A header a tool wanted and an exploit are different
claims. Say which one you have.
- **Anything in a dependency**, unless ProjectSend's use of it is what makes it reachable. Those
belong upstream, and Dependabot already watches for them here.
## Supported versions
| | |
|---|---|
| **ProjectSend 2.x** | Supported. Fixes land on the current release line; upgrade before reporting that an older 2.x behaves differently. |
| **The companion packages** — [`community-modules`](https://github.com/projectsend/community-modules), [`v1-migration-tool`](https://github.com/projectsend/v1-migration-tool) | Supported, on their own version lines. Report them here or on their own repository; both reach the same people. |
| **ProjectSend Legacy (v1)** | A separate application in a separate repository — see [projectsend/legacy](https://github.com/projectsend/legacy) for how it handles reports. Nothing here applies to it. |
## Hardening your own installation
If you are trying to configure an installation rather than report a bug, the deployment
documentation is what you want: [INSTALL.md](INSTALL.md) for a manual install, including the web
server rules that keep uploaded files private, and [DOCKER.md](DOCKER.md) for the image, where
those rules are already in place.
Two things are worth knowing whichever way you installed:
- **Put TLS in front of it**, and set `TRUSTED_PROXIES` when you do. Without it every visitor
appears to come from the proxy, which turns the login rate limiter into one bucket for all of
them and records the wrong address in the download log.
- **`APP_KEY` decrypts what is already stored.** Back it up with the database, and do not rotate it
on a running installation without knowing what you are doing.
// A living account: the stock unique message said all there
// is to say.
$fail('validation.unique')->translate();
return;
}
if($holder->erase_after!==null){
$fail(__('This email address belongs to a deleted account that is scheduled for permanent erasure. The address becomes available on :date. To free it sooner, erase the account with the projectsend:erase-account console command.',[
// Deleted before erasure scheduling existed, so no purge will ever
// reach it — only the operator command can free the address.
$fail(__('This email address belongs to a deleted account that has no erasure scheduled. Run the projectsend:erase-account console command to erase it and free the address.'));
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
ignacionelson
