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.
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.
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 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.
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.
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.
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.
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.
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.
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.
ignacionelson