colin
/
bookstack
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Images: Changed how new image permissions are set 

Removed default public visibility for images at the driver level,
leaving only doing this as a specific action in the logic.
Added try/catch around permission setting so that
permission-incompatible environments won't fatally fail, but instead
log a warning.

Tested via a google cloud storage bucket FUSE mount, mounted under another
user but with open 777 permissions.

Related to #5269
This commit is contained in:
Dan Brown 2025-05-03 20:30:50 +01:00
parent fa566f156a
commit 1262083fcf
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
2 changed files with 14 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/Config/filesystems.php
View File

@ -32,7 +32,6 @@ return [
'local' => [ 'local' => [
'driver' => 'local', 'driver' => 'local',
'root' => public_path(), 'root' => public_path(),
'visibility' => 'public',
'serve' => false, 'serve' => false,
'throw' => true, 'throw' => true,
], ],
@ -47,7 +46,6 @@ return [
'local_secure_images' => [ 'local_secure_images' => [
'driver' => 'local', 'driver' => 'local',
'root' => storage_path('uploads/images/'), 'root' => storage_path('uploads/images/'),
'visibility' => 'public',
'serve' => false, 'serve' => false,
'throw' => true, 'throw' => true,
], ],

17
app/Uploads/ImageStorageDisk.php
View File

@ -5,6 +5,8 @@ namespace BookStack\Uploads;
use BookStack\Util\FilePathNormalizer; use BookStack\Util\FilePathNormalizer;
use Illuminate\Contracts\Filesystem\Filesystem; use Illuminate\Contracts\Filesystem\Filesystem;
use Illuminate\Filesystem\FilesystemAdapter; use Illuminate\Filesystem\FilesystemAdapter;
use Illuminate\Support\Facades\Log;
use League\Flysystem\UnableToSetVisibility;
use Symfony\Component\HttpFoundation\StreamedResponse; use Symfony\Component\HttpFoundation\StreamedResponse;
class ImageStorageDisk class ImageStorageDisk
@ -74,12 +76,19 @@ class ImageStorageDisk
$path = $this->adjustPathForDisk($path); $path = $this->adjustPathForDisk($path);
$this->filesystem->put($path, $data); $this->filesystem->put($path, $data);
// Set visibility when a non-AWS-s3, s3-like storage option is in use. // Set public visibility to ensure public access on S3, or that the file is accessible
// Done since this call can break s3-like services but desired for other image stores. // to other processes (like web-servers) for local file storage options.
// Attempting to set ACL during above put request requires different permissions // We avoid attempting this for (non-AWS) s3-like systems (even in a try-catch) as
// hence would technically be a breaking change for actual s3 usage. // we've always avoided setting permissions for s3-like due to potential issues,
// with docs advising setting pre-configured permissions instead.
// We also don't do this as the default filesystem/driver level as that can technically
// require different ACLs for S3, and this provides us more logical control.
if ($makePublic && !$this->isS3Like()) { if ($makePublic && !$this->isS3Like()) {
try {
$this->filesystem->setVisibility($path, 'public'); $this->filesystem->setVisibility($path, 'public');
} catch (UnableToSetVisibility $e) {
Log::warning("Unable to set visibility for image upload with relative path: {$path}");
}
} }
} }