mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-21 19:13:20 +00:00
6ad26bb61e
Reported by @ry2811 as GHSA-6jh6-gvj5-pv8v. A resumable upload declares its size, and that declaration is what store() weighs against the maximum file size and the client's storage quota. Only the assembled file was ever held to it. The parts in between were bounded one request at a time and never added up, so a client could declare one byte and then stream parts: ten thousand part numbers at twice a 20 MB part is about 400 GB, per session, and the number of sessions was not bounded either. None of it counted against anything, because nothing becomes a File row until the upload completes and ClientStorageUsage sums File rows. A client with a 1 MB quota could fill the volume and repeat. putPart()'s own comment described this defect and treated the per-part cap as the answer to it: "without a cap here the exposure is a day's worth of disk". A cap on one request bounds one request. The exposure was a day's worth of disk multiplied by however many requests somebody cared to make. Three limits, and each one exists because the other two do not cover it. A session may not stage more than it declared. The room for a part is claimed before the body is read — a body's length is not known until it has arrived, and by then it is on the disk being protected — and the write is then capped at exactly what was claimed, so an over-long body is cut off mid-stream as it always was, against a smaller number. The claim is a read and a conditional update under a per-session lock, the same shape complete() already uses: the protocol sends parts in parallel and how many is the client's choice, so an unlocked read lets every part in flight claim the same room, while an atomic claim alone refuses the honest parallel upload instead. Whatever the part really weighs is settled back afterwards, in a finally, or a client's own retries would exhaust a session with room to spare. Open sessions count against the quota at the size they declared. A quota measured against finished files alone is spent twice by opening sessions one after another — each is told there is room, because the ones before it have not finished. The cost is that an abandoned transfer holds its share until it is cancelled or swept, so the sweeper now runs hourly rather than daily: that gap is now somebody unable to upload, which it was not before. And a cap on open sessions, because for anyone with no quota to spend — staff, and clients on an installation that sets none — the session count is the only thing between a declared size and any multiple of it. Four tests fail on the unfixed code, and three existing ones had to change: they declared a tiny size and sent a large part deliberately, to reach the re-checks at complete(). That route is now closed at putPart(), so they reach those re-checks the way a real install would instead — the file-size limit or the quota moving while a long transfer is running, which is the reason complete() re-asks rather than trusting what store() decided. The staged-byte total is BIGINT UNSIGNED, and the suite runs SQLite, which has no unsigned integers. The first version of the bounds read `staged_bytes + :delta BETWEEN 0 AND size` and raised SQLSTATE 22003 on MySQL for any refund — in the comparison, so the bound written to prevent the underflow was the statement that underflowed. Every SQLite test passed on it. Both bounds are now arranged so the column is never inside a subtraction, and UploadSessionStagedBytesMysqlTest skips loudly unless the connection is MySQL. Verified against 8.4, as was the report itself: three sessions declaring one byte each put 6 MB on the volume of a client with a 1 MB quota before, and nothing at all after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CNFU55Tkq6MuEQ73nbbBRx
386 lines
16 KiB
PHP
386 lines
16 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
use App\Models\User;
|
|
use App\Modules\Clients\ClientStorageUsage;
|
|
use App\Modules\Files\Models\File;
|
|
use App\Modules\Files\Uploads\UploadSession;
|
|
use App\Modules\Identity\Models\RolePermission;
|
|
use App\Modules\Identity\Permissions\Permission;
|
|
use App\Modules\Platform\Settings\Setting;
|
|
use App\Modules\Platform\Settings\Settings;
|
|
use Illuminate\Http\UploadedFile;
|
|
use Illuminate\Support\Facades\Storage;
|
|
use Illuminate\Support\Str;
|
|
use Inertia\Testing\AssertableInertia;
|
|
|
|
beforeEach(function () {
|
|
Storage::fake('files');
|
|
$this->admin = User::factory()->create();
|
|
});
|
|
|
|
function grantUploadPermission(User $client): void
|
|
{
|
|
RolePermission::query()->firstOrCreate(['role_id' => $client->role_id, 'permission' => Permission::Upload->value]);
|
|
}
|
|
|
|
function createChunkedSession(int $size, string $filename = 'chunked.zip'): string
|
|
{
|
|
$response = test()->postJson('/uploads', [
|
|
'filename' => $filename,
|
|
'size' => $size,
|
|
'type' => 'application/octet-stream',
|
|
]);
|
|
|
|
$response->assertOk();
|
|
|
|
return $response->json('uploadId');
|
|
}
|
|
|
|
function putChunkedPart(string $sessionId, int $part, string $content): void
|
|
{
|
|
$sign = test()->getJson("/uploads/{$sessionId}/parts/{$part}/sign")->assertOk();
|
|
test()->call('PUT', $sign->json('url'), [], [], [], ['CONTENT_TYPE' => 'application/octet-stream'], $content);
|
|
}
|
|
|
|
function makeClientFile(User $client, int $sizeBytes, string $name = 'existing'): File
|
|
{
|
|
return File::factory()->create([
|
|
'uploaded_by' => $client->id,
|
|
'name' => $name,
|
|
'original_name' => $name.'.pdf',
|
|
'path' => '2026/07/'.Str::uuid()->toString().'.pdf',
|
|
'mime_type' => 'application/pdf',
|
|
'size' => $sizeBytes,
|
|
]);
|
|
}
|
|
|
|
// The simple single-request client upload endpoint (POST /my-files/upload)
|
|
// was removed once the client portal moved to the same chunked upload
|
|
// mechanism staff uses — see "a client under quota can create and
|
|
// complete a chunked session", "...rejected at session creation",
|
|
// "...rejected at completion", and "a quota of 0 is unlimited for
|
|
// chunked uploads too" below for this file's equivalent coverage.
|
|
|
|
test('usage sums only that client\'s own non-deleted files', function () {
|
|
$client = User::factory()->client()->create();
|
|
$other = User::factory()->client()->create();
|
|
|
|
$kept = makeClientFile($client, 1024 * 1024, 'kept');
|
|
makeClientFile($client, 2 * 1024 * 1024, 'deleted')->delete();
|
|
makeClientFile($other, 5 * 1024 * 1024, 'someone-elses');
|
|
|
|
$usage = app(ClientStorageUsage::class);
|
|
|
|
expect($usage->usedBytes($client))->toBe($kept->size);
|
|
});
|
|
|
|
test('the default storage quota setting prefills a new client\'s quota field', function () {
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 500);
|
|
|
|
$this->actingAs($this->admin)->get('/clients/create')->assertInertia(
|
|
fn (AssertableInertia $page) => $page->where('default_storage_quota_mb', 500),
|
|
);
|
|
});
|
|
|
|
test('both client screens show the quota that will actually be enforced, floor included', function () {
|
|
// The screens present this as what happens, not as a value being
|
|
// edited, so they have to show the effective number. The edit screen
|
|
// in particular mirrors quotaMb()'s resolution client-side to draw the
|
|
// usage bar: handed the raw setting on a floored installation, it
|
|
// computes an effective quota of 0, prints "unlimited", and hides the
|
|
// bar entirely — for a client whose next upload is about to be
|
|
// rejected for exceeding a limit the screen said did not exist.
|
|
config()->set('projectsend.platform.default_client_quota_mb', 2048);
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 0);
|
|
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 0]);
|
|
|
|
$this->actingAs($this->admin)->get('/clients/create')->assertInertia(
|
|
fn (AssertableInertia $page) => $page->where('default_storage_quota_mb', 2048),
|
|
);
|
|
|
|
$this->actingAs($this->admin)->get("/clients/{$client->id}")->assertInertia(
|
|
fn (AssertableInertia $page) => $page->where('default_storage_quota_mb', 2048),
|
|
);
|
|
});
|
|
|
|
test('the settings form still edits the stored setting, never the floor', function () {
|
|
// The other half, and the reason this is not one change applied
|
|
// everywhere. That field is read, then written back on save. Prefilled
|
|
// with the floor, the next save of that page would write the
|
|
// platform's number into the setting as the administrator's own
|
|
// choice — where it would outlive the floor being removed.
|
|
config()->set('projectsend.platform.default_client_quota_mb', 2048);
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 0);
|
|
|
|
$this->actingAs($this->admin)->get('/system/settings/clients')->assertInertia(
|
|
fn (AssertableInertia $page) => $page->where('default_client_storage_quota_mb', 0),
|
|
);
|
|
});
|
|
|
|
test('clearing the storage quota field to blank on the edit form resets it to inherit the site default', function () {
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 150);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 100]);
|
|
|
|
$this->actingAs($this->admin)->patch("/clients/{$client->id}", [
|
|
'name' => $client->name,
|
|
'email' => $client->email,
|
|
'active' => true,
|
|
'storage_quota_mb' => '',
|
|
])->assertRedirect()->assertSessionDoesntHaveErrors();
|
|
|
|
$client->refresh();
|
|
expect($client->storage_quota_mb)->toBe(0)
|
|
->and(app(ClientStorageUsage::class)->quotaMb($client))->toBe(150);
|
|
});
|
|
|
|
test('a client under quota can create and complete a chunked session', function () {
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 1]);
|
|
grantUploadPermission($client);
|
|
$this->actingAs($client);
|
|
|
|
$sessionId = createChunkedSession(11, 'small.txt');
|
|
putChunkedPart($sessionId, 1, 'hello-');
|
|
putChunkedPart($sessionId, 2, 'world');
|
|
|
|
$this->postJson("/uploads/{$sessionId}/complete")->assertOk();
|
|
|
|
expect(File::query()->where('uploaded_by', $client->id)->exists())->toBeTrue();
|
|
});
|
|
|
|
test('a chunked session whose declared size already exceeds quota is rejected at session creation', function () {
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 1]);
|
|
grantUploadPermission($client);
|
|
makeClientFile($client, 1000 * 1024); // ~1000 KB already used, out of a 1 MB quota
|
|
|
|
$this->actingAs($client);
|
|
$this->postJson('/uploads', [
|
|
'filename' => 'huge.pdf',
|
|
'size' => 200 * 1024,
|
|
'type' => 'application/pdf',
|
|
])->assertJsonValidationErrors('size');
|
|
|
|
expect(UploadSession::query()->count())->toBe(0);
|
|
});
|
|
|
|
test('a client whose quota fills while the transfer is running is rejected at completion', function () {
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 1]);
|
|
grantUploadPermission($client);
|
|
$this->actingAs($client);
|
|
|
|
// Declared honestly, and there is room for it when the session opens.
|
|
$sessionId = createChunkedSession(50 * 1024, 'honest.txt');
|
|
putChunkedPart($sessionId, 1, str_repeat('a', 50 * 1024));
|
|
|
|
// The rest of the quota goes while the bytes are in flight — another
|
|
// device, a staff member uploading on their behalf, a second transfer
|
|
// finishing first. A big file takes a long time and the check at
|
|
// session creation is only true of the moment it was made, which is
|
|
// why completion asks again rather than trusting it.
|
|
makeClientFile($client, 1000 * 1024);
|
|
|
|
$this->postJson("/uploads/{$sessionId}/complete")->assertJsonValidationErrors('size');
|
|
|
|
expect(File::query()->where('uploaded_by', $client->id)->count())->toBe(1) // only the file that filled the quota
|
|
->and(UploadSession::query()->find($sessionId))->toBeNull();
|
|
|
|
$paths = Storage::disk('files')->allFiles();
|
|
expect(collect($paths)->filter(fn (string $path) => str_ends_with($path, '.txt')))->toBeEmpty();
|
|
});
|
|
|
|
test('a quota of 0 is unlimited for chunked uploads too, when no site default is set', function () {
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 0);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 0]);
|
|
grantUploadPermission($client);
|
|
makeClientFile($client, 500 * 1024 * 1024);
|
|
$this->actingAs($client);
|
|
|
|
$sessionId = createChunkedSession(11, 'another.txt');
|
|
putChunkedPart($sessionId, 1, 'hello-');
|
|
putChunkedPart($sessionId, 2, 'world');
|
|
|
|
$this->postJson("/uploads/{$sessionId}/complete")->assertOk();
|
|
});
|
|
|
|
test('ClientStorageUsage::quotaMb() resolves a client with no custom quota to the site default', function () {
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 250);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 0]);
|
|
|
|
expect(app(ClientStorageUsage::class)->quotaMb($client))->toBe(250);
|
|
});
|
|
|
|
test('a platform floor holds a client the installation never gave a quota to', function () {
|
|
// The hole this closes: the setting's own default is 0, 0 means
|
|
// unlimited, and an account that arrived without an explicit quota --
|
|
// a self-registered one, say -- inherits it. On an installation a
|
|
// platform runs for other people that is unmetered hosting one account
|
|
// away, so a platform may put a floor under it from the environment,
|
|
// exactly as it sets the seat caps.
|
|
config()->set('projectsend.platform.default_client_quota_mb', 1);
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 0);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 0]);
|
|
grantUploadPermission($client);
|
|
makeClientFile($client, 1000 * 1024);
|
|
$this->actingAs($client);
|
|
|
|
$this->postJson('/uploads', [
|
|
'filename' => 'over-the-floor.pdf',
|
|
'size' => 200 * 1024,
|
|
'type' => 'application/pdf',
|
|
])->assertJsonValidationErrors('size');
|
|
});
|
|
|
|
test('a floor is under the setting, never over it', function () {
|
|
// An administrator who has chosen a number keeps it, including a
|
|
// larger one. The floor is for the installation that chose nothing.
|
|
config()->set('projectsend.platform.default_client_quota_mb', 100);
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 250);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 0]);
|
|
|
|
expect(app(ClientStorageUsage::class)->quotaMb($client))->toBe(250);
|
|
});
|
|
|
|
test('an install with no platform behind it is unaffected', function () {
|
|
// Nothing set anywhere is still unlimited. This must not become a
|
|
// ceiling that appears on self-hosted installs by default.
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 0);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 0]);
|
|
|
|
expect(app(ClientStorageUsage::class)->quotaMb($client))->toBe(0);
|
|
});
|
|
|
|
test('ClientStorageUsage::quotaMb() lets a client\'s own custom quota override the site default', function () {
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 250);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 500]);
|
|
|
|
expect(app(ClientStorageUsage::class)->quotaMb($client))->toBe(500);
|
|
});
|
|
|
|
test('a client with no custom quota is limited by the site default once one is set', function () {
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 1);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 0]);
|
|
grantUploadPermission($client);
|
|
makeClientFile($client, 1000 * 1024); // ~1000 KB already used, out of the 1 MB site default
|
|
$this->actingAs($client);
|
|
|
|
$this->postJson('/uploads', [
|
|
'filename' => 'over-the-default.pdf',
|
|
'size' => 200 * 1024,
|
|
'type' => 'application/pdf',
|
|
])->assertJsonValidationErrors('size');
|
|
});
|
|
|
|
test('the rejection names the quota the client is actually held to', function () {
|
|
// storage_quota_mb of 0 means "no quota of their own", and the site
|
|
// default is what is then enforced — so the column is the one number
|
|
// the message must not print.
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 1);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 0]);
|
|
grantUploadPermission($client);
|
|
makeClientFile($client, 1000 * 1024);
|
|
$this->actingAs($client);
|
|
|
|
$response = $this->postJson('/uploads', [
|
|
'filename' => 'over-the-default.pdf',
|
|
'size' => 200 * 1024,
|
|
'type' => 'application/pdf',
|
|
])->assertJsonValidationErrors('size');
|
|
|
|
expect($response->json('errors.size.0'))->toBe('This upload would exceed your storage quota of 1 MB.');
|
|
});
|
|
|
|
test('the same is true when the real byte count is what pushes them over', function () {
|
|
// The completion check is a second copy of the same sentence, and had
|
|
// the same bug.
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 1);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 0]);
|
|
grantUploadPermission($client);
|
|
$this->actingAs($client);
|
|
|
|
$sessionId = createChunkedSession(50 * 1024, 'honest.txt');
|
|
putChunkedPart($sessionId, 1, str_repeat('a', 50 * 1024));
|
|
|
|
makeClientFile($client, 1000 * 1024);
|
|
|
|
$response = $this->postJson("/uploads/{$sessionId}/complete")->assertJsonValidationErrors('size');
|
|
|
|
expect($response->json('errors.size.0'))->toBe('This upload would exceed your storage quota of 1 MB.');
|
|
});
|
|
|
|
test('a client with a quota of their own still sees their own number', function () {
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 250);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 1]);
|
|
grantUploadPermission($client);
|
|
makeClientFile($client, 1000 * 1024);
|
|
$this->actingAs($client);
|
|
|
|
$response = $this->postJson('/uploads', [
|
|
'filename' => 'over-their-own.pdf',
|
|
'size' => 200 * 1024,
|
|
'type' => 'application/pdf',
|
|
])->assertJsonValidationErrors('size');
|
|
|
|
expect($response->json('errors.size.0'))->toBe('This upload would exceed your storage quota of 1 MB.');
|
|
});
|
|
|
|
test('a client\'s own custom quota is unaffected by later changes to the site default', function () {
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 1);
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 500]); // an explicit, much larger override
|
|
grantUploadPermission($client);
|
|
makeClientFile($client, 1000 * 1024); // already past the 1 MB site default, but well under this client's own 500 MB
|
|
$this->actingAs($client);
|
|
|
|
$sessionId = createChunkedSession(200 * 1024, 'still-fine.pdf');
|
|
putChunkedPart($sessionId, 1, str_repeat('a', 200 * 1024));
|
|
|
|
$this->postJson("/uploads/{$sessionId}/complete")->assertOk();
|
|
});
|
|
|
|
test('a self-registered client with no explicit quota inherits the site default automatically', function () {
|
|
app(Settings::class)->set(Setting::ClientsCanRegister, true);
|
|
app(Settings::class)->set(Setting::ClientsAutoApprove, true);
|
|
app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 1);
|
|
|
|
$this->post('/register', [
|
|
'name' => 'Self Registered',
|
|
'email' => 'self-registered@example.com',
|
|
'password' => 'super-secret-password',
|
|
'password_confirmation' => 'super-secret-password',
|
|
])->assertRedirect();
|
|
|
|
$client = User::query()->where('email', 'self-registered@example.com')->sole();
|
|
expect($client->storage_quota_mb)->toBe(0); // never set explicitly — inherits at enforcement time
|
|
|
|
grantUploadPermission($client);
|
|
$this->actingAs($client);
|
|
|
|
// Well within the 1 MB site default up front, then a second upload
|
|
// that would push the client over it — the flood scenario this
|
|
// feature exists to close.
|
|
makeClientFile($client, 900 * 1024);
|
|
|
|
$this->postJson('/uploads', [
|
|
'filename' => 'flood-attempt.pdf',
|
|
'size' => 200 * 1024,
|
|
'type' => 'application/pdf',
|
|
])->assertJsonValidationErrors('size');
|
|
});
|
|
|
|
test('a staff upload into a client\'s folder is unaffected by that client\'s quota', function () {
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 1]);
|
|
makeClientFile($client, 1000 * 1024); // already near the 1 MB quota
|
|
|
|
$this->actingAs($this->admin)->post('/files', [
|
|
'file' => UploadedFile::fake()->create('staff-upload.pdf', 500, 'application/pdf'),
|
|
'name' => '',
|
|
'description' => '',
|
|
])->assertRedirect()->assertSessionDoesntHaveErrors();
|
|
|
|
// The staff upload is attributed to the staff member, not the client,
|
|
// so it never counts against the client's quota.
|
|
$staffFile = File::query()->where('original_name', 'staff-upload.pdf')->sole();
|
|
expect($staffFile->uploaded_by)->toBe($this->admin->id);
|
|
});
|