fix(file-browser): preserve group permissions for shared storage

core/src/Support/FileBrowserAccess.php

@@ -80,4 +80,143 @@ public static function resolveUploadRoot(
 
         return null;
     }
+
+    /**
+     * Find the site-side spelling of a configured upload URL, without trusting it as a grant.
+     * Callers must still compare its real directory with the configured physical upload root.
+     * Remote URLs, traversal and encoded paths cannot establish an ACL identity.
+     *
+     * @return string|null Absolute site-side path, which may end at a deployment symlink.
+     * @since 3.6.0
+     */
+    public static function localUploadPath(string $url, string $basePath, string $baseUrl, string $siteUrl): ?string
+    {
+        $parts = parse_url($url);
+        $site = parse_url($siteUrl);
+        if ($parts === false || $site === false || isset($parts['query']) || isset($parts['fragment'])
+            || isset($parts['user']) || isset($parts['pass'])
+            || (isset($parts['scheme']) && !in_array(strtolower($parts['scheme']), ['http', 'https'], true))
+            || (isset($parts['host']) && (strcasecmp($parts['host'], $site['host'] ?? '') !== 0
+                || ($parts['port'] ?? null) !== ($site['port'] ?? null)))) {
+            return null;
+        }
+
+        $path = $parts['path'] ?? '';
+        if (strpbrk($path, "\\%\0:") !== false || in_array('..', explode('/', $path), true)
+            || \fileManagerRefuseReservedName($path)) {
+            return null;
+        }
+        $baseUrl = '/' . trim($baseUrl, '/');
+        if (str_starts_with($path, '/')) {
+            if ($baseUrl !== '/' && !FileManagerAccess::isWithin($baseUrl, $path)) {
+                return null;
+            }
+            $path = $baseUrl === '/' ? ltrim($path, '/') : ltrim(substr($path, strlen($baseUrl)), '/');
+        }
+
+        $root = realpath($basePath);
+        if ($root === false) {
+            return null;
+        }
+        $segments = array_filter(explode('/', $path), static fn ($part) => $part !== '' && $part !== '.');
+
+        return rtrim(str_replace('\\', '/', $root), '/') . '/' . implode('/', $segments);
+    }
+
+    /**
+     * Keep file_groups keys stable when a deployment exposes a configured shared type root.
+     * Only the type root may be an alias: the browser must separately reject every symlink
+     * below it. The site-side root is accepted only when realpath proves it is the same folder.
+     * No matching root returns null; callers must not interpret that as a public file.
+     *
+     * @return string|null Key relative to the site-wide ACL root; an empty key is that root itself.
+     * @since 3.6.0
+     */
+    public static function resolveAclPath(string $aclRoot, string $typeDir, string $absolutePath, ?string $localTypeDir = null): ?string
+    {
+        foreach ([$aclRoot, $typeDir, $absolutePath, $localTypeDir ?? ''] as $path) {
+            if (strpos($path, "\0") !== false || \fileManagerRefuseReservedName($path)) {
+                return null;
+            }
+        }
+        $root = realpath($aclRoot);
+        $typeRoot = realpath($typeDir);
+        if ($root === false || $typeRoot === false) {
+            return null;
+        }
+        $root = rtrim(str_replace('\\', '/', $root), '/');
+        $typeRoot = rtrim(str_replace('\\', '/', $typeRoot), '/');
+        $typeDir = rtrim(str_replace('\\', '/', $typeDir), '/');
+        $absolutePath = rtrim(str_replace('\\', '/', $absolutePath), '/');
+        $spelling = FileManagerAccess::isWithin($typeDir, $absolutePath) ? $typeDir : $typeRoot;
+        if (!FileManagerAccess::isWithin($spelling, $absolutePath)) {
+            return null;
+        }
+        $suffix = ltrim(substr($absolutePath, strlen($spelling)), '/');
+        if (in_array('..', explode('/', $suffix), true) || strpos($suffix, "\0") !== false
+            || \fileManagerPathContainsLink($typeDir, $absolutePath)) {
+            return null;
+        }
+        $existing = realpath($absolutePath);
+        if ($existing !== false) {
+            $existing = str_replace('\\', '/', $existing);
+            if (!FileManagerAccess::isWithin($typeRoot, $existing)) {
+                return null;
+            }
+            $suffix = ltrim(substr($existing, strlen($typeRoot)), '/');
+        } else {
+            $suffix = implode('/', array_filter(explode('/', $suffix), static fn ($part) => $part !== '' && $part !== '.'));
+        }
+
+        if (FileManagerAccess::isWithin($root, $typeRoot)) {
+            $key = ltrim(substr($typeRoot, strlen($root)), '/');
+        } elseif ($localTypeDir !== null && FileManagerAccess::isWithin($root, $localTypeDir)
+            && ($localRoot = realpath($localTypeDir)) !== false
+            && FileManagerAccess::isWithin($typeRoot, str_replace('\\', '/', $localRoot))
+            && FileManagerAccess::isWithin(str_replace('\\', '/', $localRoot), $typeRoot)) {
+            $key = ltrim(substr($localTypeDir, strlen($root)), '/');
+        } else {
+            return null;
+        }
+
+        return trim($key . '/' . $suffix, '/');
+    }
+
+    /**
+     * Diagnose an upload root without opening it to an unrecognised ACL namespace.
+     * External shared roots need a proven site-side identity for every existing browser type.
+     * This read-only check never creates folders or replaces the browser's operation guards.
+     *
+     * @since 3.6.0
+     */
+    public static function isUsableRoot(string $uploadDir, string $uploadUrl, string $aclRoot, string $basePath, string $baseUrl, string $siteUrl): bool
+    {
+        $uploadDir = str_replace('[(base_path)]', $basePath, $uploadDir);
+        $aclRoot = str_replace('[(base_path)]', $basePath, $aclRoot);
+        if (strpos($uploadDir . $aclRoot, "\0") !== false
+            || \fileManagerRefuseReservedName($uploadDir) || \fileManagerRefuseReservedName($aclRoot)) {
+            return false;
+        }
+        if (!is_dir($uploadDir) || !is_readable($uploadDir) || !is_dir($aclRoot) || !is_readable($aclRoot)) {
+            return false;
+        }
+        if (self::resolveAclPath($aclRoot, $uploadDir, $uploadDir) !== null) {
+            return true;
+        }
+
+        $found = false;
+        foreach (['files', 'images', 'file', 'image', 'media'] as $type) {
+            $typeDir = rtrim($uploadDir, '/\\') . '/' . $type;
+            if (!is_dir($typeDir)) {
+                continue;
+            }
+            $found = true;
+            $local = self::localUploadPath(rtrim($uploadUrl, '/') . '/' . $type, $basePath, $baseUrl, $siteUrl);
+            if (self::resolveAclPath($aclRoot, $typeDir, $typeDir, $local) === null) {
+                return false;
+            }
+        }
+
+        return $found;
+    }
 }

core/tests/Fixtures/shared-browser-access.php

@@ -0,0 +1,223 @@
+<?php
+
+/** Exercise the real browser guards with release symlinks, SQLite ACLs and an editor session. */
+$tmp = sys_get_temp_dir() . '/evo-shared-browser-' . bin2hex(random_bytes(6));
+foreach (['release/assets/local', 'release/assets/cache', 'shared/assets/images/private', 'shared/assets/images/public', 'shared/assets/files', 'outside'] as $dir) {
+    mkdir($tmp . '/' . $dir, 0777, true);
+}
+define('EVO_BASE_PATH', $tmp . '/release/');
+define('EVO_BASE_URL', '/');
+define('EVO_SITE_URL', 'https://example.test/');
+define('EVO_MANAGER_PATH', EVO_BASE_PATH . 'manager/');
+define('EVO_CORE_PATH', EVO_BASE_PATH . 'core/');
+define('IN_MANAGER_MODE', true);
+define('IN_INSTALL_MODE', false);
+define('EVO_API_MODE', false);
+
+/**
+ * Minimal CMS host for the browser; file-group decisions use the real implementation.
+ *
+ * @since 3.6.0
+ */
+final class SharedBrowserStub
+{
+    public array $configGlobal = [];
+    public array $config = [];
+    public array $log = [];
+
+    /**
+     * Return only the settings needed by the isolated ACL checks.
+     *
+     * @since 3.6.0
+     */
+    public function getConfig($key, $default = null)
+    {
+        return $this->config[$key] ?? ['filemanager_path' => EVO_BASE_PATH, 'use_udperms' => 1][$key] ?? $default;
+    }
+
+    /**
+     * Keep the dashboard checksum check outside this filesystem regression fixture.
+     *
+     * @since 3.6.0
+     */
+    public function getManagerApi(): self
+    {
+        return $this;
+    }
+
+    /**
+     * The isolated site has no tracked CMS checksum changes.
+     *
+     * @since 3.6.0
+     */
+    public function checkSystemChecksum(): string
+    {
+        return '0';
+    }
+
+    /**
+     * Capture refusals for assertions without writing a CMS event log.
+     *
+     * @since 3.6.0
+     */
+    public function logEvent($id, $type, $message, $source): void
+    {
+        $this->log[] = $message;
+    }
+}
+
+$GLOBALS['evo'] = new SharedBrowserStub();
+/**
+ * Supply the fixture CMS host to the unmodified browser helpers.
+ *
+ * @since 3.6.0
+ */
+function evo() { return $GLOBALS['evo']; }
+/**
+ * Supply the legacy spelling used by the refusal logger.
+ *
+ * @since 3.6.0
+ */
+function evolutionCMS() { return evo(); }
+
+require dirname(__DIR__, 2) . '/vendor/autoload.php';
+$browserDir = dirname(__DIR__, 3) . '/manager/media/browser/mcpuk/';
+require $browserDir . 'lib/helper_path.php';
+require $browserDir . 'lib/helper_dir.php';
+require $browserDir . 'core/uploader.php';
+require $browserDir . 'core/browser.php';
+
+try {
+    if (!@symlink($tmp . '/shared/assets/images', EVO_BASE_PATH . 'assets/images')
+        || !@symlink($tmp . '/shared/assets/files', EVO_BASE_PATH . 'assets/files')) {
+        echo json_encode(['skip' => 'symlinks are not available']);
+        return;
+    }
+    file_put_contents($tmp . '/shared/assets/images/private/secret.txt', 'secret');
+    file_put_contents($tmp . '/shared/assets/images/public/open.txt', 'open');
+    symlink($tmp . '/outside', $tmp . '/shared/assets/images/escape');
+    symlink($tmp . '/shared/assets/images/private', $tmp . '/shared/assets/images/internal');
+    symlink($tmp . '/outside/missing', $tmp . '/shared/assets/images/dangling');
+    mkdir($tmp . '/shared/assets/.thumbs/images', 0777, true);
+    symlink($tmp . '/outside', $tmp . '/shared/assets/.thumbs-link');
+
+    $capsule = new Illuminate\Database\Capsule\Manager();
+    $capsule->addConnection(['driver' => 'sqlite', 'database' => ':memory:', 'prefix' => '']);
+    $capsule->setAsGlobal();
+    $capsule->bootEloquent();
+    $capsule->getConnection()->statement('CREATE TABLE file_groups (id INTEGER PRIMARY KEY, document_group INTEGER, file TEXT)');
+    $capsule->getConnection()->table('file_groups')->insert([
+        'id' => 1, 'document_group' => 7, 'file' => 'assets/images/private',
+    ]);
+
+    $browser = (new ReflectionClass(browser::class))->newInstanceWithoutConstructor();
+    $typeDir = $tmp . '/shared/assets/images';
+    foreach (['modx' => evo(), 'typeDir' => $typeDir, 'typeURL' => 'https://example.test/assets/images',
+        'type' => 'images', 'config' => ['uploadDir' => $tmp . '/shared/assets']] as $name => $value) {
+        (new ReflectionProperty(uploader::class, $name))->setValue($browser, $value);
+    }
+    (new ReflectionProperty(browser::class, 'protectedPaths'))->setValue($browser, []);
+    $invoke = static fn ($name, ...$args) => (new ReflectionMethod(browser::class, $name))->invoke($browser, ...$args);
+    $_SESSION = ['mgrRole' => 2, 'mgrDocgroups' => []];
+    $secret = $typeDir . '/private/secret.txt';
+    $open = $typeDir . '/public/open.txt';
+    $output = [
+        'key' => $invoke('getFileGroupsRelPath', $secret),
+        'newKey' => $invoke('getFileGroupsRelPath', $typeDir . '/private/new.txt'),
+        'dottedKey' => $invoke('getFileGroupsRelPath', $typeDir . '/./private/secret.txt'),
+        'deniedRead' => $invoke('isPathAccessible', $secret),
+        'deniedWrite' => $invoke('isWriteAllowed', 'private'),
+        'publicRead' => $invoke('isPathAccessible', $open),
+        'publicWrite' => $invoke('isWriteAllowed', 'public'),
+        'listing' => array_map('basename', $invoke('filterAccessiblePaths', [$secret, $open])),
+        'subtreeSecret' => $invoke('subtreeAccessFilter', $typeDir)($secret),
+        'protectedDescendants' => $invoke('hasInaccessibleDescendants', $typeDir),
+        'escape' => $invoke('isPathAccessible', $typeDir . '/escape'),
+        'internal' => $invoke('isPathAccessible', $typeDir . '/internal/secret.txt'),
+        'visibleLink' => $invoke('isLinkVisible', $typeDir . '/escape'),
+        'dangling' => $invoke('isWriteAllowed', 'dangling'),
+        'traversal' => $invoke('isPathAccessible', $typeDir . '/../files'),
+        'safeThumbs' => !fileManagerPathContainsLink($tmp . '/shared/assets', $tmp . '/shared/assets/.thumbs/images'),
+        'linkedThumbs' => !fileManagerPathContainsLink($tmp . '/shared/assets', $tmp . '/shared/assets/.thumbs-link/images'),
+        'diagnostic' => EvolutionCMS\Support\FileBrowserAccess::isUsableRoot(
+            $tmp . '/shared/assets', 'assets/', EVO_BASE_PATH, EVO_BASE_PATH, '/', EVO_SITE_URL
+        ),
+        'missingDiagnostic' => EvolutionCMS\Support\FileBrowserAccess::isUsableRoot(
+            $tmp . '/missing', 'assets/', EVO_BASE_PATH, EVO_BASE_PATH, '/', EVO_SITE_URL
+        ),
+        'unmappedDiagnostic' => EvolutionCMS\Support\FileBrowserAccess::isUsableRoot(
+            $tmp . '/shared/assets', 'unmapped/', EVO_BASE_PATH, EVO_BASE_PATH, '/', EVO_SITE_URL
+        ),
+        'standardDiagnostic' => EvolutionCMS\Support\FileBrowserAccess::isUsableRoot(
+            '[(base_path)]assets/', 'assets/', '[(base_path)]', EVO_BASE_PATH, '/', EVO_SITE_URL
+        ),
+        'standardKey' => EvolutionCMS\Support\FileBrowserAccess::resolveAclPath(
+            EVO_BASE_PATH, EVO_BASE_PATH . 'assets/local', EVO_BASE_PATH . 'assets/local/new.txt'
+        ),
+        'wrongTargetKey' => EvolutionCMS\Support\FileBrowserAccess::resolveAclPath(
+            EVO_BASE_PATH, $typeDir, $open, EVO_BASE_PATH . 'assets/local'
+        ),
+        'unconfinedKey' => EvolutionCMS\Support\FileBrowserAccess::resolveAclPath(
+            EVO_BASE_PATH, $typeDir, $open, $typeDir
+        ),
+        'personalKey' => EvolutionCMS\Support\FileBrowserAccess::resolveAclPath(
+            EVO_BASE_PATH, $typeDir . '/private', $secret, EVO_BASE_PATH . 'assets/images/private'
+        ),
+    ];
+    $_SESSION['mgrDocgroups'] = [7];
+    $output['memberRead'] = $invoke('isPathAccessible', $secret);
+    $output['memberWrite'] = $invoke('isWriteAllowed', 'private');
+    $output['memberSubtree'] = $invoke('subtreeAccessFilter', $typeDir)($secret);
+    $output['memberDescendants'] = $invoke('hasInaccessibleDescendants', $typeDir);
+
+    (new ReflectionProperty(uploader::class, 'typeURL'))->setValue($browser, 'https://example.test/unmapped/images');
+    $output['unmappedKey'] = $invoke('getFileGroupsRelPath', $open);
+    $output['unmappedRead'] = $invoke('isPathAccessible', $open);
+    $output['unmappedWrite'] = $invoke('isWriteAllowed', 'public');
+    $output['unmappedListing'] = $invoke('filterAccessiblePaths', [$open]);
+    $output['unmappedSubtree'] = $invoke('subtreeAccessFilter', $typeDir)($open);
+    $output['unmappedDescendants'] = $invoke('hasInaccessibleDescendants', $typeDir);
+    $output['unmappedLink'] = $invoke('isLinkVisible', $typeDir . '/escape');
+    $output['log'] = evo()->log;
+    $_SESSION['mgrRole'] = 1;
+    $output['adminRead'] = $invoke('isPathAccessible', $open);
+    $output['adminEscape'] = $invoke('isPathAccessible', $typeDir . '/escape');
+    // Another release must retain the same stored key, without moving file_groups rows.
+    mkdir($tmp . '/next-release/assets', 0777, true);
+    symlink($typeDir, $tmp . '/next-release/assets/images');
+    $output['nextReleaseKey'] = EvolutionCMS\Support\FileBrowserAccess::resolveAclPath(
+        $tmp . '/next-release', $typeDir, $secret, $tmp . '/next-release/assets/images'
+    );
+
+    // Execute the actual dashboard include, including its independent file-manager check.
+    $capsule->getConnection()->statement('CREATE TABLE site_content (id INTEGER, published INTEGER, privateweb INTEGER, deleted INTEGER)');
+    require dirname(__DIR__, 2) . '/lang/en/global.php';
+    foreach ([
+        'shared' => [$tmp . '/shared/assets/', 'assets/', '[(base_path)]'],
+        'portable' => ['[(base_path)]assets/', 'assets/', '[(base_path)]'],
+        'missing' => [$tmp . '/missing/', 'assets/', '[(base_path)]'],
+        'unmapped' => [$tmp . '/shared/assets/', 'unmapped/', '[(base_path)]'],
+        'missingManager' => ['[(base_path)]assets/', 'assets/', '[(base_path)]missing/'],
+        'outsideManager' => [$tmp . '/shared/assets/', 'assets/', $tmp . '/shared/'],
+        'traversalManager' => ['[(base_path)]assets/', 'assets/', '[(base_path)]../shared/'],
+    ] as $case => [$upload, $url, $managerRoot]) {
+        evo()->config = ['rb_base_dir' => $upload, 'rb_base_url' => $url, 'filemanager_path' => $managerRoot];
+        include dirname(__DIR__, 3) . '/manager/includes/config_check.inc.php';
+        $messages = array_column($warnings, 0);
+        $output['dashboard'][$case] = [
+            'browserWarning' => in_array($_lang['configcheck_rb_base_dir'], $messages, true),
+            'managerWarning' => in_array($_lang['configcheck_filemanager_path'], $messages, true),
+        ];
+    }
+    echo json_encode($output);
+} finally {
+    $items = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($tmp, FilesystemIterator::SKIP_DOTS), RecursiveIteratorIterator::CHILD_FIRST);
+    foreach ($items as $item) {
+        if ($item->isLink() || !$item->isDir()) {
+            unlink($item->getPathname());
+        } else {
+            rmdir($item->getPathname());
+        }
+    }
+    rmdir($tmp);
+}

core/tests/Unit/Manager/FileManagerAclSourceTest.php

@@ -4,6 +4,7 @@
 
 use Tests\TestCase;
 
+/** Source guards for ACL entry points and the ownership of persisted file-group rows. */
 final class FileManagerAclSourceTest extends TestCase
 {
     public function testClassicManagerSourceEnforcesAclOnGroupSaveAndView(): void
@@ -73,6 +74,7 @@ public function testKcfinderChecksFileGroupsOnEveryReadAndChangeOfANamedEntry():
         }
     }
 
+    /** Assert both managers use the site-wide ACL root and carry rows across entry changes. */
     public function testFileGroupsAreKeyedBySiteRootAndFollowEveryRenameAndDelete(): void
     {
         $classic = (string) file_get_contents(dirname(__DIR__, 4) . '/manager/actions/files.dynamic.php');
@@ -84,7 +86,8 @@ public function testFileGroupsAreKeyedBySiteRootAndFollowEveryRenameAndDelete():
         self::assertStringContainsString("\$groupsKey = \$groupsTarget === null ? null : fileManagerAclKey(\$groupsTargetPath);", $classic);
         self::assertStringContainsString("FileGroup::query()->where('file', \$groupsKey)->get();", $classic);
         self::assertStringNotContainsString("where('file', \$groupsTargetPath)", $classic);
-        self::assertStringContainsString('\EvolutionCMS\Support\FileManagerAccess::aclRoot(),', $browser);
+        // The global root may be stored in a variable before the shared-path mapping.
+        self::assertStringContainsString('\EvolutionCMS\Support\FileManagerAccess::aclRoot()', $browser);
         self::assertStringNotContainsString("getConfig('filemanager_path', EVO_BASE_PATH)", $browser);
 
         // rows follow the entry whoever renames or deletes it, not only group managers