From adfc07d741c320f746b0ed5590379c20e4ff0fab Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 17:54:10 +0000 Subject: [PATCH 01/39] feat(types): add symlink fields to FileOrFolder interface --- frontend/src/shared.types.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frontend/src/shared.types.ts b/frontend/src/shared.types.ts index 35eaef59..7648d5f1 100644 --- a/frontend/src/shared.types.ts +++ b/frontend/src/shared.types.ts @@ -9,6 +9,11 @@ type FileOrFolder = { last_modified: number; hasRead?: boolean; hasWrite?: boolean; + is_symlink?: boolean; + symlink_target_fsp?: { + fsp_name: string; + subpath: string; + } | null; }; type FileSharePath = { From fed7ad1b40a08ee54018b5da053084de3c117fa0 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 17:54:13 +0000 Subject: [PATCH 02/39] feat(filestore): add symlink detection and target resolution to FileInfo - Add is_symlink and symlink_target_fsp fields to FileInfo model - Implement symlink detection in from_stat method using lstat - Resolve symlink targets to file share paths when session is provided - Add session parameter to get_file_info, yield_file_infos, and helper methods --- fileglancer/filestore.py | 82 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 74 insertions(+), 8 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index 32f45341..501b0a0a 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -44,9 +44,12 @@ class FileInfo(BaseModel): last_modified: Optional[float] = None hasRead: Optional[bool] = None hasWrite: Optional[bool] = None + is_symlink: bool = False + symlink_target_fsp: Optional[dict] = None # {"fsp_name": str, "subpath": str} @classmethod - def from_stat(cls, path: str, absolute_path: str, stat_result: os.stat_result, current_user: str = None): + def from_stat(cls, path: str, absolute_path: str, stat_result: os.stat_result, + current_user: str = None, session = None): """Create FileInfo from os.stat_result""" if path is None or path == "": raise ValueError("Path cannot be None or empty") @@ -76,6 +79,27 @@ def from_stat(cls, path: str, absolute_path: str, stat_result: os.stat_result, c hasRead = cls._has_read_permission(stat_result, current_user, owner, group) hasWrite = cls._has_write_permission(stat_result, current_user, owner, group) + # Use lstat to detect symlinks without following them + lstat_result = os.lstat(absolute_path) + is_symlink = stat.S_ISLNK(lstat_result.st_mode) + symlink_target_fsp = None + + if is_symlink and session is not None: + # Read the symlink target + target = os.readlink(absolute_path) + + # Resolve to absolute path (relative symlinks need dirname context) + if not os.path.isabs(target): + target = os.path.join(os.path.dirname(absolute_path), target) + target = os.path.abspath(target) + + # Find which file share contains this target + from fileglancer.database import find_fsp_from_absolute_path + match = find_fsp_from_absolute_path(session, target) + if match: + fsp, subpath = match + symlink_target_fsp = {"fsp_name": fsp.name, "subpath": subpath} + return cls( name=name, path=path, @@ -87,7 +111,9 @@ def from_stat(cls, path: str, absolute_path: str, stat_result: os.stat_result, c group=group, last_modified=last_modified, hasRead=hasRead, - hasWrite=hasWrite + hasWrite=hasWrite, + is_symlink=is_symlink, + symlink_target_fsp=symlink_target_fsp ) @staticmethod @@ -190,7 +216,7 @@ def _check_path_in_root(self, path: Optional[str]) -> str: return full_path - def _get_file_info_from_path(self, full_path: str, current_user: str = None) -> FileInfo: + def _get_file_info_from_path(self, full_path: str, current_user: str = None, session = None) -> FileInfo: """ Get the FileInfo for a file or directory at the given path. """ @@ -202,7 +228,7 @@ def _get_file_info_from_path(self, full_path: str, current_user: str = None) -> rel_path = '.' else: rel_path = os.path.relpath(full_real, root_real) - return FileInfo.from_stat(rel_path, full_path, stat_result, current_user) + return FileInfo.from_stat(rel_path, full_path, stat_result, current_user, session) def get_root_path(self) -> str: @@ -228,7 +254,7 @@ def get_absolute_path(self, relative_path: Optional[str] = None) -> str: return os.path.abspath(os.path.join(self.root_path, relative_path)) - def get_file_info(self, path: Optional[str] = None, current_user: str = None) -> FileInfo: + def get_file_info(self, path: Optional[str] = None, current_user: str = None, session = None) -> FileInfo: """ Get the FileInfo for a file or directory at the given path. @@ -237,12 +263,50 @@ def get_file_info(self, path: Optional[str] = None, current_user: str = None) -> May be None, in which case the root directory is used. current_user (str): The username of the current user for permission checking. May be None, in which case hasRead and hasWrite will be None. + session: Database session for symlink resolution. + May be None, in which case symlink_target_fsp will be None. + + Raises: + ValueError: If path attempts to escape root directory + """ + full_path = self._check_path_in_root(path) + return self._get_file_info_from_path(full_path, current_user, session) + + + def check_is_binary(self, path: Optional[str] = None, sample_size: int = 4096) -> bool: + """ + Check if a file is likely binary by reading a sample of its contents. + + Args: + path (str): The relative path to the file to check. + May be None, in which case the root is checked (always returns False for directories). + sample_size (int): Number of bytes to read for binary detection. Defaults to 4096. + + Returns: + bool: True if the file appears to be binary, False otherwise. + Returns False for directories. Raises: ValueError: If path attempts to escape root directory + FileNotFoundError: If the file does not exist + PermissionError: If the file cannot be read """ + from .utils import is_likely_binary + full_path = self._check_path_in_root(path) - return self._get_file_info_from_path(full_path, current_user) + + # Directories are not binary + if os.path.isdir(full_path): + return False + + try: + with open(full_path, 'rb') as f: + sample = f.read(sample_size) + return is_likely_binary(sample) + except Exception as e: + # If we can't read the file, assume it's binary to be safe + logger.warning(f"Could not read file sample for binary detection: {e}") + return True def check_is_binary(self, path: Optional[str] = None, sample_size: int = 4096) -> bool: @@ -281,7 +345,7 @@ def check_is_binary(self, path: Optional[str] = None, sample_size: int = 4096) - return True - def yield_file_infos(self, path: Optional[str] = None, current_user: str = None) -> Generator[FileInfo, None, None]: + def yield_file_infos(self, path: Optional[str] = None, current_user: str = None, session = None) -> Generator[FileInfo, None, None]: """ Yield a FileInfo object for each child of the given path. @@ -290,6 +354,8 @@ def yield_file_infos(self, path: Optional[str] = None, current_user: str = None) May be None, in which case the root directory is listed. current_user (str): The username of the current user for permission checking. May be None, in which case hasRead and hasWrite will be None. + session: Database session for symlink resolution. + May be None, in which case symlink_target_fsp will be None for symlinks. Raises: PermissionError: If the path is not accessible due to permissions. @@ -304,7 +370,7 @@ def yield_file_infos(self, path: Optional[str] = None, current_user: str = None) for entry in entries: entry_path = os.path.join(full_path, entry) try: - yield self._get_file_info_from_path(entry_path, current_user) + yield self._get_file_info_from_path(entry_path, current_user, session) except (FileNotFoundError, PermissionError, OSError) as e: logger.error(f"Error accessing entry: {entry_path}: {e}") continue From bed9f73e5b062e9615f48b8dda4112806e5882fe Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 17:54:15 +0000 Subject: [PATCH 03/39] feat(ui): display symlink type in file table --- .../src/components/ui/BrowsePage/fileTableColumns.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/ui/BrowsePage/fileTableColumns.tsx b/frontend/src/components/ui/BrowsePage/fileTableColumns.tsx index 34327ae4..7ef13884 100644 --- a/frontend/src/components/ui/BrowsePage/fileTableColumns.tsx +++ b/frontend/src/components/ui/BrowsePage/fileTableColumns.tsx @@ -10,9 +10,13 @@ import { formatUnixTimestamp, formatFileSize } from '@/utils'; export const typeColumn: ColumnDef = { accessorKey: 'is_dir', header: 'Type', - cell: ({ getValue }) => ( - {getValue() ? 'Folder' : 'File'} - ), + cell: ({ row }) => { + const file = row.original; + if (file.is_symlink) { + return Symlink; + } + return {file.is_dir ? 'Folder' : 'File'}; + }, sortingFn: (rowA, rowB) => { const a = rowA.original.is_dir; const b = rowB.original.is_dir; From 242f4b32880bd1baf3f454581d81c440bbd4cfe1 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 17:54:18 +0000 Subject: [PATCH 04/39] feat(app): pass database session to filestore for symlink resolution Update /api/files endpoint to open a database session and pass it to filestore methods, enabling symlink target resolution. --- fileglancer/app.py | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/fileglancer/app.py b/fileglancer/app.py index d3a01aa2..711b0337 100644 --- a/fileglancer/app.py +++ b/fileglancer/app.py @@ -1274,27 +1274,28 @@ async def get_file_metadata(path_name: str, subpath: Optional[str] = Query(''), raise HTTPException(status_code=404 if "not found" in error else 500, detail=error) try: - file_info = filestore.get_file_info(subpath, username) - logger.trace(f"File info: {file_info}") - - result = {"info": json.loads(file_info.model_dump_json())} - - if file_info.is_dir: - try: - files = list(filestore.yield_file_infos(subpath, username)) - result["files"] = [json.loads(f.model_dump_json()) for f in files] - except PermissionError: - logger.error(f"Permission denied when listing files in directory: {subpath}") - result["files"] = [] - result["error"] = "Permission denied when listing directory contents" - return JSONResponse(content=result, status_code=403) - except FileNotFoundError: - logger.error(f"Directory not found during listing: {subpath}") - result["files"] = [] - result["error"] = "Directory contents not found" - return JSONResponse(content=result, status_code=404) - - return result + with db.get_db_session(settings.db_url) as session: + file_info = filestore.get_file_info(subpath, current_user=username, session=session) + logger.trace(f"File info: {file_info}") + + result = {"info": json.loads(file_info.model_dump_json())} + + if file_info.is_dir: + try: + files = list(filestore.yield_file_infos(subpath, current_user=username, session=session)) + result["files"] = [json.loads(f.model_dump_json()) for f in files] + except PermissionError: + logger.error(f"Permission denied when listing files in directory: {subpath}") + result["files"] = [] + result["error"] = "Permission denied when listing directory contents" + return JSONResponse(content=result, status_code=403) + except FileNotFoundError: + logger.error(f"Directory not found during listing: {subpath}") + result["files"] = [] + result["error"] = "Directory contents not found" + return JSONResponse(content=result, status_code=404) + + return result except RootCheckError as e: # Path attempts to escape root directory - try to find a valid fsp for this absolute path From 86dfaaccdfbc05427ef211ee3511148637d0a0a7 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 17:54:20 +0000 Subject: [PATCH 05/39] feat(ui): add symlink icon and navigation support --- .../src/components/ui/BrowsePage/FileTable.tsx | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/frontend/src/components/ui/BrowsePage/FileTable.tsx b/frontend/src/components/ui/BrowsePage/FileTable.tsx index a9dfa2a4..f972bcba 100644 --- a/frontend/src/components/ui/BrowsePage/FileTable.tsx +++ b/frontend/src/components/ui/BrowsePage/FileTable.tsx @@ -9,7 +9,7 @@ import { type SortingState } from '@tanstack/react-table'; import { IconButton, Typography } from '@material-tailwind/react'; -import { TbFile } from 'react-icons/tb'; +import { TbFile, TbLink } from 'react-icons/tb'; import { HiOutlineEllipsisHorizontalCircle, HiOutlineFolder @@ -60,7 +60,14 @@ export default function Table({ const name = getValue() as string; let link = '#'; - if (file.is_dir && fileQuery.data?.currentFileSharePath) { + if (file.is_symlink && file.symlink_target_fsp) { + // Symlink with known target in a valid file share + link = makeBrowseLink( + file.symlink_target_fsp.fsp_name, + file.symlink_target_fsp.subpath + ) as string; + } else if (file.is_dir && fileQuery.data?.currentFileSharePath) { + // Regular directory link = makeBrowseLink( fileQuery.data?.currentFileSharePath.name, file.path @@ -69,13 +76,15 @@ export default function Table({ return (
- {file.is_dir ? ( + {file.is_symlink ? ( + + ) : file.is_dir ? ( ) : ( )} - {file.is_dir ? ( + {file.is_dir || file.is_symlink ? ( {name} From 6e734155b2d63f84456430464ff3850b4c7b3be9 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 17:58:13 +0000 Subject: [PATCH 06/39] test(filestore): add symlink support tests Add 7 comprehensive pytest test cases for symlink support: - Symlink detection and basic properties - Same-share symlink resolution via directory listing - Cross-share symlink resolution via directory listing - Relative symlink resolution - Symlink listing in directory (yield_file_infos) - Broken symlink handling (filtered from listings) - Symlink to directory detection All tests use mocked database sessions and verify: - is_symlink flag is correctly set - symlink_target_fsp contains correct fsp_name and subpath - Broken symlinks are skipped in directory listings - Symlinks to directories preserve is_dir=True --- tests/test_filestore.py | 241 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 241 insertions(+) diff --git a/tests/test_filestore.py b/tests/test_filestore.py index a7933106..4cda813f 100644 --- a/tests/test_filestore.py +++ b/tests/test_filestore.py @@ -172,3 +172,244 @@ def test_change_file_permissions_invalid_permissions(filestore): def test_change_file_permissions_invalid_path(filestore): with pytest.raises(ValueError): filestore.change_file_permissions("nonexistent.txt", "rw-r--r--") + + +# Symlink tests + +def test_symlink_detection(test_dir): + """Test that FileInfo correctly detects symlinks and their properties""" + # Create a file and a symlink to it + target_file = os.path.join(test_dir, "target.txt") + with open(target_file, "w") as f: + f.write("symlink target content") + + symlink_path = os.path.join(test_dir, "link_to_target") + os.symlink(target_file, symlink_path) + + # Get FileInfo using stat (follow symlink for stat_result) + # Note: pass the symlink path as absolute_path so lstat can detect it + stat_result = os.stat(symlink_path) + file_info = FileInfo.from_stat("link_to_target", symlink_path, stat_result) + + assert file_info.is_symlink is True + assert file_info.name == "link_to_target" + + +def test_same_share_symlink_resolution_via_listing(filestore, test_dir): + """Test symlink resolution when target is within the same file share via directory listing""" + # Create target file + target_file = os.path.join(test_dir, "subdir", "target_same_share.txt") + with open(target_file, "w") as f: + f.write("same share target content") + + # Create symlink to target at root of test_dir + symlink_path = os.path.join(test_dir, "link_to_subdir_file") + os.symlink(target_file, symlink_path) + + # Get file info via directory listing with session (mock database session) + from unittest.mock import Mock + from fileglancer import database + + mock_session = Mock() + original_find = database.find_fsp_from_absolute_path + + def mock_find(session, path): + # Normalize paths for comparison + if os.path.realpath(path) == os.path.realpath(target_file): + fsp = FileSharePath(zone="test", name="test", mount_path=test_dir) + return (fsp, "subdir/target_same_share.txt") + return None + + database.find_fsp_from_absolute_path = mock_find + + try: + # Use yield_file_infos to list directory - symlinks are detected this way + files = list(filestore.yield_file_infos("", session=mock_session)) + symlink_info = next((f for f in files if f.name == "link_to_subdir_file"), None) + + assert symlink_info is not None + assert symlink_info.is_symlink is True + assert symlink_info.symlink_target_fsp is not None + assert symlink_info.symlink_target_fsp["fsp_name"] == "test" + assert symlink_info.symlink_target_fsp["subpath"] == "subdir/target_same_share.txt" + finally: + database.find_fsp_from_absolute_path = original_find + + +def test_cross_share_symlink_resolution_via_listing(test_dir): + """Test symlink resolution when target is in a different file share via directory listing""" + # Create two file shares + share1_dir = os.path.join(test_dir, "share1") + share2_dir = os.path.join(test_dir, "share2") + os.makedirs(share1_dir) + os.makedirs(share2_dir) + + # Create target in share2 + target_file = os.path.join(share2_dir, "target.txt") + with open(target_file, "w") as f: + f.write("cross-share target") + + # Create symlink in share1 pointing to share2 + symlink_path = os.path.join(share1_dir, "link_to_share2") + os.symlink(target_file, symlink_path) + + # Create filestore for share1 + fsp1 = FileSharePath(zone="test", name="share1", mount_path=share1_dir) + filestore1 = Filestore(fsp1) + + # Mock database session and find function + from unittest.mock import Mock + from fileglancer import database + + mock_session = Mock() + original_find = database.find_fsp_from_absolute_path + + def mock_find(session, path): + # Normalize paths for comparison + if os.path.realpath(path) == os.path.realpath(target_file): + fsp2 = FileSharePath(zone="test", name="share2", mount_path=share2_dir) + return (fsp2, "target.txt") + return None + + database.find_fsp_from_absolute_path = mock_find + + try: + # Use yield_file_infos to list directory - symlinks are detected this way + files = list(filestore1.yield_file_infos("", session=mock_session)) + symlink_info = next((f for f in files if f.name == "link_to_share2"), None) + + assert symlink_info is not None + assert symlink_info.is_symlink is True + assert symlink_info.symlink_target_fsp is not None + assert symlink_info.symlink_target_fsp["fsp_name"] == "share2" + assert symlink_info.symlink_target_fsp["subpath"] == "target.txt" + finally: + database.find_fsp_from_absolute_path = original_find + + +def test_relative_symlink_resolution(test_dir): + """Test that relative symlinks are resolved correctly""" + # Create a fresh directory structure for this test + nested_dir = os.path.join(test_dir, "rel_test", "nested") + os.makedirs(nested_dir, exist_ok=True) + target_file = os.path.join(test_dir, "rel_test", "target.txt") + with open(target_file, "w") as f: + f.write("relative target") + + # Create relative symlink from nested directory pointing up + symlink_path = os.path.join(nested_dir, "link") + os.symlink("../target.txt", symlink_path) + + # Create filestore for nested_dir so symlink is listed via yield_file_infos + fsp = FileSharePath(zone="test", name="nested", mount_path=nested_dir) + nested_filestore = Filestore(fsp) + + from unittest.mock import Mock + from fileglancer import database + + mock_session = Mock() + original_find = database.find_fsp_from_absolute_path + + def mock_find(session, path): + if os.path.realpath(path) == os.path.realpath(target_file): + # Return fsp for rel_test directory + fsp_rel = FileSharePath(zone="test", name="rel_test", mount_path=os.path.join(test_dir, "rel_test")) + return (fsp_rel, "target.txt") + return None + + database.find_fsp_from_absolute_path = mock_find + + try: + # List directory to find the symlink + files = list(nested_filestore.yield_file_infos("", session=mock_session)) + symlink_info = next((f for f in files if f.name == "link"), None) + + assert symlink_info is not None + assert symlink_info.is_symlink is True + assert symlink_info.symlink_target_fsp is not None + assert symlink_info.symlink_target_fsp["subpath"] == "target.txt" + finally: + database.find_fsp_from_absolute_path = original_find + + +def test_yield_file_infos_with_symlinks(filestore, test_dir): + """Test that yield_file_infos correctly lists symlinks""" + # Create file and symlink + with open(os.path.join(test_dir, "file1.txt"), "w") as f: + f.write("file 1") + + os.symlink( + os.path.join(test_dir, "file1.txt"), + os.path.join(test_dir, "link1") + ) + + from unittest.mock import Mock + from fileglancer import database + + mock_session = Mock() + original_find = database.find_fsp_from_absolute_path + database.find_fsp_from_absolute_path = lambda s, p: ( + FileSharePath(zone="test", name="test", mount_path=test_dir), + "file1.txt" + ) + + try: + files = list(filestore.yield_file_infos("", session=mock_session)) + + # Find the symlink in the list + symlink_info = next((f for f in files if f.name == "link1"), None) + assert symlink_info is not None + assert symlink_info.is_symlink is True + finally: + database.find_fsp_from_absolute_path = original_find + + +def test_broken_symlink_not_listed(filestore, test_dir): + """Test that broken symlinks are not listed (caught by OSError)""" + # Create a broken symlink + broken_link = os.path.join(test_dir, "broken_link") + os.symlink("/nonexistent/path", broken_link) + + # Create a valid file for comparison + with open(os.path.join(test_dir, "valid_file.txt"), "w") as f: + f.write("valid") + + # List directory - broken symlink should not appear + files = list(filestore.yield_file_infos("")) + file_names = [f.name for f in files] + + assert "valid_file.txt" in file_names + assert "broken_link" not in file_names # Broken symlink should be skipped + + +def test_symlink_to_directory(filestore, test_dir): + """Test symlink pointing to a directory is detected via listing""" + # Create a directory + target_dir = os.path.join(test_dir, "target_dir") + os.makedirs(target_dir) + + # Create symlink to directory + symlink_path = os.path.join(test_dir, "link_to_dir") + os.symlink(target_dir, symlink_path) + + from unittest.mock import Mock + from fileglancer import database + + mock_session = Mock() + original_find = database.find_fsp_from_absolute_path + database.find_fsp_from_absolute_path = lambda s, p: ( + FileSharePath(zone="test", name="test", mount_path=test_dir), + "target_dir" + ) + + try: + # Use yield_file_infos to list directory - symlinks to dirs are detected this way + files = list(filestore.yield_file_infos("", session=mock_session)) + symlink_info = next((f for f in files if f.name == "link_to_dir"), None) + + assert symlink_info is not None + assert symlink_info.is_symlink is True + assert symlink_info.is_dir is True # Should also be marked as directory + assert symlink_info.symlink_target_fsp is not None + finally: + database.find_fsp_from_absolute_path = original_find From bb20c7831f7dd4b2f114a40734bdfb8b6df1075b Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 18:35:25 +0000 Subject: [PATCH 07/39] test(e2e): add Playwright tests for symlink navigation and display Add comprehensive E2E tests for symlink support: - Display symlink icon and type in file table - Navigate to symlink target within same share - Directory symlinks display as Symlink type (not Folder) - Context menu works on symlinks - Broken symlinks are not displayed - Selection shows symlink in properties panel Uses custom Playwright fixtures to create symlink test data. --- .../ui-tests/tests/symlink-navigation.spec.ts | 207 ++++++++++++++++++ 1 file changed, 207 insertions(+) create mode 100644 frontend/ui-tests/tests/symlink-navigation.spec.ts diff --git a/frontend/ui-tests/tests/symlink-navigation.spec.ts b/frontend/ui-tests/tests/symlink-navigation.spec.ts new file mode 100644 index 00000000..1afff2d2 --- /dev/null +++ b/frontend/ui-tests/tests/symlink-navigation.spec.ts @@ -0,0 +1,207 @@ +import { expect, test as base } from '@playwright/test'; +import { join } from 'path'; +import { existsSync } from 'fs'; +import { rm, mkdir, writeFile, symlink } from 'fs/promises'; +import { navigateToScratchFsp, navigateToTestDir } from '../utils/navigation'; +import { mockAPI, teardownMockAPI } from '../mocks/api'; +import { cleanDatabase } from '../utils/db-cleanup'; +import { randomBytes } from 'crypto'; + +import type { Page } from '@playwright/test'; + +type SymlinkFixtures = { + fileglancerPage: Page; + testDir: string; +}; + +/** + * Create symlink test fixtures in the given directory. + * Creates: + * - target_subdir/ (a directory) + * - target_subdir/target_file.txt (a file) + * - symlink_to_subdir -> target_subdir (symlink to directory) + * - symlink_to_file -> target_subdir/target_file.txt (symlink to file) + * - broken_symlink -> /nonexistent/path (broken symlink) + * - regular_file.txt (a regular file for comparison) + */ +async function createSymlinkTestFixtures(testDir: string): Promise { + // Create target directory + const targetSubdir = join(testDir, 'target_subdir'); + await mkdir(targetSubdir, { recursive: true }); + + // Create target file inside target directory + await writeFile( + join(targetSubdir, 'target_file.txt'), + 'This is the target file content' + ); + + // Create symlink to directory (same share) + await symlink(targetSubdir, join(testDir, 'symlink_to_subdir')); + + // Create symlink to file + await symlink( + join(targetSubdir, 'target_file.txt'), + join(testDir, 'symlink_to_file') + ); + + // Create broken symlink - pointing to nonexistent path + await symlink('/nonexistent/path/that/does/not/exist', join(testDir, 'broken_symlink')); + + // Create a regular file for comparison + await writeFile(join(testDir, 'regular_file.txt'), 'Regular file content'); + + console.log('[Symlink Fixture] Created symlink test fixtures in', testDir); +} + +const openFileglancer = async (page: Page) => { + await page.goto('/', { + waitUntil: 'domcontentloaded' + }); + await page.waitForSelector('text=Log In', { timeout: 10000 }); + + const loginForm = page.getByRole('textbox', { name: 'Username' }); + const loginSubmitBtn = page.getByRole('button', { name: 'Log In' }); + await loginForm.fill('testUser'); + await loginSubmitBtn.click(); + + await page.waitForSelector('text=Zones', { timeout: 10000 }); +}; + +/** + * Custom Playwright fixture for symlink tests. + * + * Similar to the main fileglancer-fixture but creates symlink test data + * instead of the standard test files and zarr directories. + */ +const test = base.extend({ + testDir: async ({}, use) => { + const scratchDir = join(global.testTempDir, 'scratch'); + const uniqueId = randomBytes(8).toString('hex'); + const testDir = `symlink-test-${uniqueId}`; + const fullPath = join(scratchDir, testDir); + + console.log(`[Symlink Fixture] Creating unique test directory: ${fullPath}`); + + // Create test directory and populate with symlink fixtures + await mkdir(fullPath, { recursive: true }); + await createSymlinkTestFixtures(fullPath); + + // Provide test directory NAME (not full path) to the test + await use(testDir); + + // Cleanup test directory after test completes + console.log(`[Symlink Fixture] Cleaning up test directory: ${fullPath}`); + if (existsSync(fullPath)) { + await rm(fullPath, { recursive: true, force: true }); + } + }, + + fileglancerPage: async ({ page, testDir }, use) => { + const fullTestPath = join(global.testTempDir, 'scratch', testDir); + console.log(`[Symlink Fixture] Test temp dir: ${global.testTempDir}`); + console.log(`[Symlink Fixture] Test directory: ${fullTestPath}`); + + // Clean user-specific database tables + cleanDatabase(global.testTempDir); + + await mockAPI(page); + await openFileglancer(page); + await navigateToScratchFsp(page); + await navigateToTestDir(page, fullTestPath); + + // Provide the page to the test + await use(page); + + // Teardown + await teardownMockAPI(page); + await page.waitForTimeout(100); + } +}); + +test.describe('Symlink Navigation and Display', () => { + test.beforeEach('Verify symlink test environment is loaded', async ({ fileglancerPage: page }) => { + // Wait for files to load - verify regular_file.txt is visible + await expect(page.getByText('regular_file.txt', { exact: true })).toBeVisible({ timeout: 10000 }); + }); + + test('displays symlink icon and type', async ({ fileglancerPage: page }) => { + // Look for a symlink in the file table - symlink_to_file should exist + const symlinkRow = page.getByRole('row').filter({ hasText: 'symlink_to_file' }); + await expect(symlinkRow).toBeVisible(); + + // Verify symlink icon is visible (TbLink icon renders as svg) + await expect(symlinkRow.locator('svg').first()).toBeVisible(); + + // Verify Type column shows "Symlink" + await expect(symlinkRow.getByText('Symlink')).toBeVisible(); + }); + + test('navigates to symlink target within same share', async ({ fileglancerPage: page }) => { + // Double-click on a symlink that points to a directory in the same file share + // Note: We use double-click since that's how directory navigation works + await page.getByRole('link', { name: 'symlink_to_subdir' }).dblclick(); + + // Wait for navigation to complete and verify we can see the target file inside the target directory + await expect(page.getByText('target_file.txt', { exact: true })).toBeVisible({ timeout: 10000 }); + }); + + test('directory symlink displays as Symlink type, not Folder', async ({ fileglancerPage: page }) => { + // The symlink to a directory should still show as Symlink type, not Folder + const symlinkDirRow = page.getByRole('row').filter({ hasText: 'symlink_to_subdir' }); + await expect(symlinkDirRow).toBeVisible(); + + // Should show "Symlink" type even though it points to a directory + await expect(symlinkDirRow.getByText('Symlink')).toBeVisible(); + + // Verify the regular directory shows as "Folder" + const regularDirRow = page.getByRole('row').filter({ hasText: 'target_subdir' }); + await expect(regularDirRow.getByText('Folder')).toBeVisible(); + }); + + test('context menu works on symlinks', async ({ fileglancerPage: page }) => { + // Right-click on the symlink text in the table to open context menu + // Following the pattern from file-operations.spec.ts + await page.getByText('symlink_to_file', { exact: true }).click({ button: 'right' }); + + // Verify context menu appears - wait for it to be visible + await expect(page.getByRole('menuitem', { name: /rename/i })).toBeVisible({ timeout: 5000 }); + await expect(page.getByRole('menuitem', { name: /delete/i })).toBeVisible(); + + // Close the menu by pressing Escape + await page.keyboard.press('Escape'); + }); + + test('broken symlinks are not displayed', async ({ fileglancerPage: page }) => { + // The broken_symlink was created pointing to /nonexistent/path + // It should NOT appear in the file list because os.stat() fails on broken symlinks + // and the backend catches that error and skips the entry + await expect(page.getByText('broken_symlink', { exact: true })).not.toBeVisible(); + + // But valid files should still be visible + await expect(page.getByText('regular_file.txt', { exact: true })).toBeVisible(); + await expect(page.getByText('symlink_to_file', { exact: true })).toBeVisible(); + }); + + test('clicking symlink row selects it and shows in properties panel', async ({ fileglancerPage: page }) => { + // Verify properties panel is visible first + const propertiesPanel = page.locator('[role="complementary"]').filter({ hasText: 'Properties' }); + await expect(propertiesPanel).toBeVisible(); + + // For symlinks, clicking the SVG navigates to the target (since symlinks are clickable) + // Instead, we click directly on the row but on the "Type" column (which says "Symlink") + // This selects the row without triggering navigation + const symlinkRow = page.getByRole('row').filter({ hasText: 'symlink_to_file' }); + await symlinkRow.getByText('Symlink').click(); + + // Wait for the properties panel to update + await page.waitForTimeout(300); + + // Verify properties panel shows the symlink name + // The name is displayed in a FgTooltip which adds aria-label attribute + await expect( + propertiesPanel.getByLabel('symlink_to_file', { exact: true }) + ).toBeVisible({ timeout: 10000 }); + }); +}); + +export { test, expect }; From 7494e8eef0af301c1d9be59ffe27f6dfac4432fc Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 14:51:41 -0500 Subject: [PATCH 08/39] fix: remove favorite and conversion request actions from symlink context menu --- frontend/src/components/ui/BrowsePage/FileBrowser.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/ui/BrowsePage/FileBrowser.tsx b/frontend/src/components/ui/BrowsePage/FileBrowser.tsx index 770c9bdd..8f298f50 100644 --- a/frontend/src/components/ui/BrowsePage/FileBrowser.tsx +++ b/frontend/src/components/ui/BrowsePage/FileBrowser.tsx @@ -134,14 +134,20 @@ export default function FileBrowser({ toast.success(`Favorite ${isFavorite ? 'removed!' : 'added!'}`); } }, - shouldShow: fileBrowserState.selectedFiles[0]?.is_dir ?? false + shouldShow: + (fileBrowserState.selectedFiles[0]?.is_dir && + !fileBrowserState.selectedFiles[0].is_symlink) ?? + false }, { name: 'Convert images to OME-Zarr', action: () => { setShowConvertFileDialog(true); }, - shouldShow: tasksEnabled && propertiesTarget.is_dir + shouldShow: + tasksEnabled && + propertiesTarget.is_dir && + !propertiesTarget.is_symlink }, { name: 'Rename', From fd3e0da82f31a05cc9d53b4b6277d2e6e908c780 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 15:57:21 -0500 Subject: [PATCH 09/39] fix: locators in display symlink icon and type test --- frontend/ui-tests/tests/symlink-navigation.spec.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/frontend/ui-tests/tests/symlink-navigation.spec.ts b/frontend/ui-tests/tests/symlink-navigation.spec.ts index 1afff2d2..6d90bc25 100644 --- a/frontend/ui-tests/tests/symlink-navigation.spec.ts +++ b/frontend/ui-tests/tests/symlink-navigation.spec.ts @@ -129,11 +129,13 @@ test.describe('Symlink Navigation and Display', () => { const symlinkRow = page.getByRole('row').filter({ hasText: 'symlink_to_file' }); await expect(symlinkRow).toBeVisible(); - // Verify symlink icon is visible (TbLink icon renders as svg) - await expect(symlinkRow.locator('svg').first()).toBeVisible(); + // Verify symlink icon is visible (TbLink icon renders as svg with text-primary class) + await expect(symlinkRow.locator('svg.text-primary')).toBeVisible(); - // Verify Type column shows "Symlink" - await expect(symlinkRow.getByText('Symlink')).toBeVisible(); + // Verify Type column shows "Symlink" (use exact match to avoid matching filename) + await expect( + symlinkRow.getByText('Symlink', { exact: true }) + ).toBeVisible(); }); test('navigates to symlink target within same share', async ({ fileglancerPage: page }) => { From 5d97426433814035c19451627ab3c8a68d6b7618 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 15:58:45 -0500 Subject: [PATCH 10/39] fix: locator in navigate to symlink target in same share test --- frontend/ui-tests/tests/symlink-navigation.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/ui-tests/tests/symlink-navigation.spec.ts b/frontend/ui-tests/tests/symlink-navigation.spec.ts index 6d90bc25..c2dc9cbc 100644 --- a/frontend/ui-tests/tests/symlink-navigation.spec.ts +++ b/frontend/ui-tests/tests/symlink-navigation.spec.ts @@ -142,9 +142,10 @@ test.describe('Symlink Navigation and Display', () => { // Double-click on a symlink that points to a directory in the same file share // Note: We use double-click since that's how directory navigation works await page.getByRole('link', { name: 'symlink_to_subdir' }).dblclick(); - - // Wait for navigation to complete and verify we can see the target file inside the target directory - await expect(page.getByText('target_file.txt', { exact: true })).toBeVisible({ timeout: 10000 }); + const targetFileRow = page + .getByRole('row') + .filter({ hasText: 'target_file' }); + await expect(targetFileRow).toBeVisible(); }); test('directory symlink displays as Symlink type, not Folder', async ({ fileglancerPage: page }) => { From cd4873b8112bcb64d8327cb89bd562c042d663b1 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 16:00:42 -0500 Subject: [PATCH 11/39] fix: locator in directory symlink displays as Symlink type test --- frontend/ui-tests/tests/symlink-navigation.spec.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/frontend/ui-tests/tests/symlink-navigation.spec.ts b/frontend/ui-tests/tests/symlink-navigation.spec.ts index c2dc9cbc..ecc5ba93 100644 --- a/frontend/ui-tests/tests/symlink-navigation.spec.ts +++ b/frontend/ui-tests/tests/symlink-navigation.spec.ts @@ -153,12 +153,18 @@ test.describe('Symlink Navigation and Display', () => { const symlinkDirRow = page.getByRole('row').filter({ hasText: 'symlink_to_subdir' }); await expect(symlinkDirRow).toBeVisible(); - // Should show "Symlink" type even though it points to a directory - await expect(symlinkDirRow.getByText('Symlink')).toBeVisible(); + // Should show "Symlink" type even though it points to a directory (use exact match) + await expect( + symlinkDirRow.getByText('Symlink', { exact: true }) + ).toBeVisible(); // Verify the regular directory shows as "Folder" - const regularDirRow = page.getByRole('row').filter({ hasText: 'target_subdir' }); - await expect(regularDirRow.getByText('Folder')).toBeVisible(); + const regularDirRow = page + .getByRole('row') + .filter({ hasText: 'target_subdir' }); + await expect( + regularDirRow.getByText('Folder', { exact: true }) + ).toBeVisible(); }); test('context menu works on symlinks', async ({ fileglancerPage: page }) => { From 7e26b76cf967590fd0f9840bc3491b12ec044561 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 16:02:08 -0500 Subject: [PATCH 12/39] fix: click on a directory to view properties panel in test; clicking on files opens them --- .../ui-tests/tests/symlink-navigation.spec.ts | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/frontend/ui-tests/tests/symlink-navigation.spec.ts b/frontend/ui-tests/tests/symlink-navigation.spec.ts index ecc5ba93..c6954ffb 100644 --- a/frontend/ui-tests/tests/symlink-navigation.spec.ts +++ b/frontend/ui-tests/tests/symlink-navigation.spec.ts @@ -191,24 +191,22 @@ test.describe('Symlink Navigation and Display', () => { await expect(page.getByText('symlink_to_file', { exact: true })).toBeVisible(); }); - test('clicking symlink row selects it and shows in properties panel', async ({ fileglancerPage: page }) => { + test('clicking a directory symlink row selects it and shows in properties panel', async ({ + fileglancerPage: page + }) => { // Verify properties panel is visible first const propertiesPanel = page.locator('[role="complementary"]').filter({ hasText: 'Properties' }); await expect(propertiesPanel).toBeVisible(); - // For symlinks, clicking the SVG navigates to the target (since symlinks are clickable) - // Instead, we click directly on the row but on the "Type" column (which says "Symlink") - // This selects the row without triggering navigation - const symlinkRow = page.getByRole('row').filter({ hasText: 'symlink_to_file' }); - await symlinkRow.getByText('Symlink').click(); - - // Wait for the properties panel to update - await page.waitForTimeout(300); + // Click on the "Type" column to select without triggering the hyperlink + const symlinkRow = page + .getByRole('row') + .filter({ hasText: 'symlink_to_subdir' }); + await symlinkRow.getByText('Symlink', { exact: true }).click(); - // Verify properties panel shows the symlink name - // The name is displayed in a FgTooltip which adds aria-label attribute + // Verify properties panel shows the symlink name in the header await expect( - propertiesPanel.getByLabel('symlink_to_file', { exact: true }) + propertiesPanel.getByText('symlink_to_subdir', { exact: true }) ).toBeVisible({ timeout: 10000 }); }); }); From 6d92cf77fa564c12f4e6dd53082a0783e7fe78d6 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 16:02:25 -0500 Subject: [PATCH 13/39] chore: prettier/eslint formatting --- .../ui-tests/tests/symlink-navigation.spec.ts | 69 +++++++++++++------ 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/frontend/ui-tests/tests/symlink-navigation.spec.ts b/frontend/ui-tests/tests/symlink-navigation.spec.ts index c6954ffb..072b5565 100644 --- a/frontend/ui-tests/tests/symlink-navigation.spec.ts +++ b/frontend/ui-tests/tests/symlink-navigation.spec.ts @@ -45,7 +45,10 @@ async function createSymlinkTestFixtures(testDir: string): Promise { ); // Create broken symlink - pointing to nonexistent path - await symlink('/nonexistent/path/that/does/not/exist', join(testDir, 'broken_symlink')); + await symlink( + '/nonexistent/path/that/does/not/exist', + join(testDir, 'broken_symlink') + ); // Create a regular file for comparison await writeFile(join(testDir, 'regular_file.txt'), 'Regular file content'); @@ -80,7 +83,9 @@ const test = base.extend({ const testDir = `symlink-test-${uniqueId}`; const fullPath = join(scratchDir, testDir); - console.log(`[Symlink Fixture] Creating unique test directory: ${fullPath}`); + console.log( + `[Symlink Fixture] Creating unique test directory: ${fullPath}` + ); // Create test directory and populate with symlink fixtures await mkdir(fullPath, { recursive: true }); @@ -119,14 +124,20 @@ const test = base.extend({ }); test.describe('Symlink Navigation and Display', () => { - test.beforeEach('Verify symlink test environment is loaded', async ({ fileglancerPage: page }) => { - // Wait for files to load - verify regular_file.txt is visible - await expect(page.getByText('regular_file.txt', { exact: true })).toBeVisible({ timeout: 10000 }); - }); + test.beforeEach( + 'Verify symlink test environment is loaded', + async ({ fileglancerPage: page }) => { + await expect( + page.getByText('regular_file.txt', { exact: true }) + ).toBeVisible({ timeout: 10000 }); + } + ); test('displays symlink icon and type', async ({ fileglancerPage: page }) => { // Look for a symlink in the file table - symlink_to_file should exist - const symlinkRow = page.getByRole('row').filter({ hasText: 'symlink_to_file' }); + const symlinkRow = page + .getByRole('row') + .filter({ hasText: 'symlink_to_file' }); await expect(symlinkRow).toBeVisible(); // Verify symlink icon is visible (TbLink icon renders as svg with text-primary class) @@ -138,9 +149,9 @@ test.describe('Symlink Navigation and Display', () => { ).toBeVisible(); }); - test('navigates to symlink target within same share', async ({ fileglancerPage: page }) => { - // Double-click on a symlink that points to a directory in the same file share - // Note: We use double-click since that's how directory navigation works + test('navigates to symlink target within same share', async ({ + fileglancerPage: page + }) => { await page.getByRole('link', { name: 'symlink_to_subdir' }).dblclick(); const targetFileRow = page .getByRole('row') @@ -148,9 +159,13 @@ test.describe('Symlink Navigation and Display', () => { await expect(targetFileRow).toBeVisible(); }); - test('directory symlink displays as Symlink type, not Folder', async ({ fileglancerPage: page }) => { + test('directory symlink displays as Symlink type, not Folder', async ({ + fileglancerPage: page + }) => { // The symlink to a directory should still show as Symlink type, not Folder - const symlinkDirRow = page.getByRole('row').filter({ hasText: 'symlink_to_subdir' }); + const symlinkDirRow = page + .getByRole('row') + .filter({ hasText: 'symlink_to_subdir' }); await expect(symlinkDirRow).toBeVisible(); // Should show "Symlink" type even though it points to a directory (use exact match) @@ -170,32 +185,46 @@ test.describe('Symlink Navigation and Display', () => { test('context menu works on symlinks', async ({ fileglancerPage: page }) => { // Right-click on the symlink text in the table to open context menu // Following the pattern from file-operations.spec.ts - await page.getByText('symlink_to_file', { exact: true }).click({ button: 'right' }); + await page + .getByText('symlink_to_file', { exact: true }) + .click({ button: 'right' }); // Verify context menu appears - wait for it to be visible - await expect(page.getByRole('menuitem', { name: /rename/i })).toBeVisible({ timeout: 5000 }); + await expect(page.getByRole('menuitem', { name: /rename/i })).toBeVisible({ + timeout: 5000 + }); await expect(page.getByRole('menuitem', { name: /delete/i })).toBeVisible(); // Close the menu by pressing Escape await page.keyboard.press('Escape'); }); - test('broken symlinks are not displayed', async ({ fileglancerPage: page }) => { + test('broken symlinks are not displayed', async ({ + fileglancerPage: page + }) => { // The broken_symlink was created pointing to /nonexistent/path // It should NOT appear in the file list because os.stat() fails on broken symlinks // and the backend catches that error and skips the entry - await expect(page.getByText('broken_symlink', { exact: true })).not.toBeVisible(); + await expect( + page.getByText('broken_symlink', { exact: true }) + ).not.toBeVisible(); // But valid files should still be visible - await expect(page.getByText('regular_file.txt', { exact: true })).toBeVisible(); - await expect(page.getByText('symlink_to_file', { exact: true })).toBeVisible(); + await expect( + page.getByText('regular_file.txt', { exact: true }) + ).toBeVisible(); + await expect( + page.getByText('symlink_to_file', { exact: true }) + ).toBeVisible(); }); test('clicking a directory symlink row selects it and shows in properties panel', async ({ fileglancerPage: page }) => { // Verify properties panel is visible first - const propertiesPanel = page.locator('[role="complementary"]').filter({ hasText: 'Properties' }); + const propertiesPanel = page + .locator('[role="complementary"]') + .filter({ hasText: 'Properties' }); await expect(propertiesPanel).toBeVisible(); // Click on the "Type" column to select without triggering the hyperlink @@ -210,5 +239,3 @@ test.describe('Symlink Navigation and Display', () => { ).toBeVisible({ timeout: 10000 }); }); }); - -export { test, expect }; From 02e081179ab0b4b870e2258dabc4ebee98931513 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 16:41:07 -0500 Subject: [PATCH 14/39] docs: add security note to _get_file_info_from_path for CodeQL flag --- fileglancer/filestore.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index 501b0a0a..d32bf28f 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -219,6 +219,15 @@ def _check_path_in_root(self, path: Optional[str]) -> str: def _get_file_info_from_path(self, full_path: str, current_user: str = None, session = None) -> FileInfo: """ Get the FileInfo for a file or directory at the given path. + + Security note: full_path comes from either: + 1. _check_path_in_root() which validates user input against the root + 2. os.path.join(verified_directory, entry) where entry is from os.listdir() + + In both cases, the path has been validated or constructed from validated + components. We pass full_path (not realpath) to from_stat so that lstat() + can detect symlinks. Symlink targets may be outside the root (cross-fileshare + symlinks), which is valid - we detect and report them without following. """ stat_result = os.stat(full_path) # Use real paths to avoid /var vs /private/var mismatches on macOS. @@ -228,6 +237,7 @@ def _get_file_info_from_path(self, full_path: str, current_user: str = None, ses rel_path = '.' else: rel_path = os.path.relpath(full_real, root_real) + # Pass original full_path so lstat() in from_stat can detect symlinks return FileInfo.from_stat(rel_path, full_path, stat_result, current_user, session) From 4e725dbedd27033770c6b874783dc91ee141ad1c Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Wed, 4 Feb 2026 16:54:48 -0500 Subject: [PATCH 15/39] Potential fix for code scanning alert no. 57: Uncontrolled data used in path expression Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- fileglancer/filestore.py | 43 +++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index d32bf28f..75cda0ab 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -47,6 +47,21 @@ class FileInfo(BaseModel): is_symlink: bool = False symlink_target_fsp: Optional[dict] = None # {"fsp_name": str, "subpath": str} + @staticmethod + def _safe_readlink(path: str) -> Optional[str]: + """ + Safely read a symlink target. + + This wrapper exists so that any filesystem access based on paths that may + originate from user input is centralized and error-tolerant. Callers are + expected to have already validated that 'path' is within an allowed root. + """ + try: + return os.readlink(path) + except OSError as e: + logger.warning(f"Failed to read symlink target for {path}: {e}") + return None + @classmethod def from_stat(cls, path: str, absolute_path: str, stat_result: os.stat_result, current_user: str = None, session = None): @@ -85,20 +100,20 @@ def from_stat(cls, path: str, absolute_path: str, stat_result: os.stat_result, symlink_target_fsp = None if is_symlink and session is not None: - # Read the symlink target - target = os.readlink(absolute_path) - - # Resolve to absolute path (relative symlinks need dirname context) - if not os.path.isabs(target): - target = os.path.join(os.path.dirname(absolute_path), target) - target = os.path.abspath(target) - - # Find which file share contains this target - from fileglancer.database import find_fsp_from_absolute_path - match = find_fsp_from_absolute_path(session, target) - if match: - fsp, subpath = match - symlink_target_fsp = {"fsp_name": fsp.name, "subpath": subpath} + # Read the symlink target safely + target = cls._safe_readlink(absolute_path) + if target is not None: + # Resolve to absolute path (relative symlinks need dirname context) + if not os.path.isabs(target): + target = os.path.join(os.path.dirname(absolute_path), target) + target = os.path.abspath(target) + + # Find which file share contains this target + from fileglancer.database import find_fsp_from_absolute_path + match = find_fsp_from_absolute_path(session, target) + if match: + fsp, subpath = match + symlink_target_fsp = {"fsp_name": fsp.name, "subpath": subpath} return cls( name=name, From d3885cd375eb513373939271e16c72d919cba50f Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 4 Feb 2026 17:03:06 -0500 Subject: [PATCH 16/39] fix: add defense-in-depth validation to symlink reading - flagged by CodeQL - Adds root_path parameter to _safe_readlink and from_stat to verify the symlink's parent directory is within the allowed root before calling os.readlink(). Uses parent directory check (not realpath) to preserve cross-share symlink support. --- fileglancer/filestore.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index 75cda0ab..2694e470 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -48,15 +48,26 @@ class FileInfo(BaseModel): symlink_target_fsp: Optional[dict] = None # {"fsp_name": str, "subpath": str} @staticmethod - def _safe_readlink(path: str) -> Optional[str]: + def _safe_readlink(path: str, root_path: Optional[str] = None) -> Optional[str]: """ Safely read a symlink target. - This wrapper exists so that any filesystem access based on paths that may - originate from user input is centralized and error-tolerant. Callers are - expected to have already validated that 'path' is within an allowed root. + This wrapper centralizes symlink reading with defense-in-depth validation. + When root_path is provided, verifies the symlink's parent directory is + within the allowed root before calling os.readlink(). This check uses + the parent directory (not realpath of the symlink itself) because + realpath would resolve the symlink to its target, which may legitimately + be outside root for cross-share symlinks. """ try: + if root_path is not None: + root_real = os.path.realpath(root_path) + # Check the symlink's parent directory is within root + # (don't resolve the symlink itself - that would check the target) + parent_real = os.path.realpath(os.path.dirname(path)) + if not (parent_real == root_real or parent_real.startswith(root_real + os.sep)): + logger.warning(f"Refusing to read symlink outside root: {path}") + return None return os.readlink(path) except OSError as e: logger.warning(f"Failed to read symlink target for {path}: {e}") @@ -64,7 +75,7 @@ def _safe_readlink(path: str) -> Optional[str]: @classmethod def from_stat(cls, path: str, absolute_path: str, stat_result: os.stat_result, - current_user: str = None, session = None): + current_user: str = None, session = None, root_path: Optional[str] = None): """Create FileInfo from os.stat_result""" if path is None or path == "": raise ValueError("Path cannot be None or empty") @@ -101,7 +112,7 @@ def from_stat(cls, path: str, absolute_path: str, stat_result: os.stat_result, if is_symlink and session is not None: # Read the symlink target safely - target = cls._safe_readlink(absolute_path) + target = cls._safe_readlink(absolute_path, root_path=root_path) if target is not None: # Resolve to absolute path (relative symlinks need dirname context) if not os.path.isabs(target): @@ -253,7 +264,8 @@ def _get_file_info_from_path(self, full_path: str, current_user: str = None, ses else: rel_path = os.path.relpath(full_real, root_real) # Pass original full_path so lstat() in from_stat can detect symlinks - return FileInfo.from_stat(rel_path, full_path, stat_result, current_user, session) + # Pass root_path for defense-in-depth validation in _safe_readlink + return FileInfo.from_stat(rel_path, full_path, stat_result, current_user, session, root_path=self.root_path) def get_root_path(self) -> str: From 8cb1a9b843aedfc5152d4938f173335e81633393 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Fri, 6 Feb 2026 21:08:02 +0000 Subject: [PATCH 17/39] feat: detect and return broken symlinks in file listings --- fileglancer/filestore.py | 160 ++++++++++++++++++++++++++++++--------- tests/test_filestore.py | 36 +++++++-- 2 files changed, 155 insertions(+), 41 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index 2694e470..9834f598 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -73,16 +73,87 @@ def _safe_readlink(path: str, root_path: Optional[str] = None) -> Optional[str]: logger.warning(f"Failed to read symlink target for {path}: {e}") return None + @staticmethod + def _get_stat_result(absolute_path: str, is_symlink: bool, lstat_result: os.stat_result) -> os.stat_result: + """ + Get the appropriate stat result for a file, handling broken symlinks. + + For symlinks, attempts to stat the target. If that fails (broken symlink), + returns the lstat result instead. + """ + if is_symlink: + try: + # Try to stat the target for file metadata + return os.stat(absolute_path) + except (FileNotFoundError, PermissionError, OSError) as e: + # Broken symlink - use lstat_result as the stat_result + logger.warning(f"Broken symlink detected: {absolute_path}: {e}") + return lstat_result + else: + # Not a symlink, stat it normally + return os.stat(absolute_path) + + @classmethod + def _get_symlink_target_fsp(cls, absolute_path: str, is_symlink: bool, session, + root_path: Optional[str]) -> Optional[dict]: + """ + Resolve a symlink target to a file share path. + + Returns a dict with fsp_name and subpath if the target is in a known file share, + or None if not a symlink, target not found, or target not in any file share. + """ + if not is_symlink or session is None: + return None + + # Read the symlink target safely + target = cls._safe_readlink(absolute_path, root_path=root_path) + if target is None: + return None + + # Resolve to absolute path (relative symlinks need dirname context) + if not os.path.isabs(target): + target = os.path.join(os.path.dirname(absolute_path), target) + target = os.path.abspath(target) + + # Try to find which file share contains this target + # If stat failed earlier (broken symlink), we won't find a match + try: + from fileglancer.database import find_fsp_from_absolute_path + match = find_fsp_from_absolute_path(session, target) + if match: + fsp, subpath = match + return {"fsp_name": fsp.name, "subpath": subpath} + except (FileNotFoundError, PermissionError, OSError): + # Target doesn't exist or isn't accessible + pass + + return None + @classmethod - def from_stat(cls, path: str, absolute_path: str, stat_result: os.stat_result, - current_user: str = None, session = None, root_path: Optional[str] = None): - """Create FileInfo from os.stat_result""" + def from_stat(cls, path: str, absolute_path: str, + lstat_result: os.stat_result, stat_result: os.stat_result, + current_user: str = None, session = None, + root_path: Optional[str] = None): + """ + Create FileInfo from pre-computed stat results. + + Args: + path: Relative path within the filestore. + absolute_path: Absolute filesystem path (used for basename and symlink resolution). + lstat_result: Result of os.lstat() on the path (detects symlinks). + stat_result: Result of os.stat() or lstat for broken symlinks. + current_user: Username for permission checking (optional). + session: Database session for symlink resolution (optional). + root_path: Filestore root for defense-in-depth validation in symlink reading (optional). + """ if path is None or path == "": raise ValueError("Path cannot be None or empty") + + is_symlink = stat.S_ISLNK(lstat_result.st_mode) is_dir = stat.S_ISDIR(stat_result.st_mode) size = 0 if is_dir else stat_result.st_size # Do not expose the name of the root directory - name = '' if path=='.' else os.path.basename(absolute_path) + name = '' if path == '.' else os.path.basename(absolute_path) permissions = stat.filemode(stat_result.st_mode) last_modified = stat_result.st_mtime @@ -105,26 +176,8 @@ def from_stat(cls, path: str, absolute_path: str, stat_result: os.stat_result, hasRead = cls._has_read_permission(stat_result, current_user, owner, group) hasWrite = cls._has_write_permission(stat_result, current_user, owner, group) - # Use lstat to detect symlinks without following them - lstat_result = os.lstat(absolute_path) - is_symlink = stat.S_ISLNK(lstat_result.st_mode) - symlink_target_fsp = None - - if is_symlink and session is not None: - # Read the symlink target safely - target = cls._safe_readlink(absolute_path, root_path=root_path) - if target is not None: - # Resolve to absolute path (relative symlinks need dirname context) - if not os.path.isabs(target): - target = os.path.join(os.path.dirname(absolute_path), target) - target = os.path.abspath(target) - - # Find which file share contains this target - from fileglancer.database import find_fsp_from_absolute_path - match = find_fsp_from_absolute_path(session, target) - if match: - fsp, subpath = match - symlink_target_fsp = {"fsp_name": fsp.name, "subpath": subpath} + # Resolve symlink target to file share path if applicable + symlink_target_fsp = cls._get_symlink_target_fsp(absolute_path, is_symlink, session, root_path) return cls( name=name, @@ -246,7 +299,7 @@ def _get_file_info_from_path(self, full_path: str, current_user: str = None, ses """ Get the FileInfo for a file or directory at the given path. - Security note: full_path comes from either: + full_path comes from either: 1. _check_path_in_root() which validates user input against the root 2. os.path.join(verified_directory, entry) where entry is from os.listdir() @@ -255,17 +308,52 @@ def _get_file_info_from_path(self, full_path: str, current_user: str = None, ses can detect symlinks. Symlink targets may be outside the root (cross-fileshare symlinks), which is valid - we detect and report them without following. """ - stat_result = os.stat(full_path) - # Use real paths to avoid /var vs /private/var mismatches on macOS. root_real = os.path.realpath(self.root_path) + + # Defense-in-depth: normalize full_path with abspath (resolves ".." + # without following symlinks) and verify it is within root before any + # filesystem operations. We use abspath (not realpath) because symlinks + # may legitimately point to targets outside root for cross-share links. + full_path = os.path.abspath(full_path) + + def _is_within_root(p: str) -> bool: + return p == root_real or p.startswith(root_real + os.sep) + + # Check the normalized path string is under root (catches .. traversal) + if not _is_within_root(full_path): + raise RootCheckError( + f"Path ({full_path}) is outside root directory ({root_real})", + full_path, + ) + + # Check the resolved parent is under root (catches symlink-based traversal + # e.g. /root/data/symlink_to_etc/passwd where symlink_to_etc -> /etc) + # Skip when full_path is the root itself, since root's parent is above root. + if full_path != root_real: + parent_real = os.path.realpath(os.path.dirname(full_path)) + if not _is_within_root(parent_real): + raise RootCheckError( + f"Path ({full_path}) resolves outside root directory ({root_real})", + full_path, + ) + full_real = os.path.realpath(full_path) if full_real == root_real: rel_path = '.' else: rel_path = os.path.relpath(full_real, root_real) - # Pass original full_path so lstat() in from_stat can detect symlinks - # Pass root_path for defense-in-depth validation in _safe_readlink - return FileInfo.from_stat(rel_path, full_path, stat_result, current_user, session, root_path=self.root_path) + + # Perform filesystem stat calls here, after validation, so that + # the sanitizer and the I/O are in the same method. + lstat_result = os.lstat(full_path) + is_symlink = stat.S_ISLNK(lstat_result.st_mode) + stat_result = FileInfo._get_stat_result(full_path, is_symlink, lstat_result) + + return FileInfo.from_stat( + rel_path, full_path, lstat_result, stat_result, + current_user=current_user, session=session, + root_path=self.root_path, + ) def get_root_path(self) -> str: @@ -304,9 +392,12 @@ def get_file_info(self, path: Optional[str] = None, current_user: str = None, se May be None, in which case symlink_target_fsp will be None. Raises: - ValueError: If path attempts to escape root directory + RootCheckError: If path attempts to escape root directory """ - full_path = self._check_path_in_root(path) + if path is None or path == "": + full_path = self.root_path + else: + full_path = os.path.join(self.root_path, path) return self._get_file_info_from_path(full_path, current_user, session) @@ -408,8 +499,9 @@ def yield_file_infos(self, path: Optional[str] = None, current_user: str = None, entry_path = os.path.join(full_path, entry) try: yield self._get_file_info_from_path(entry_path, current_user, session) - except (FileNotFoundError, PermissionError, OSError) as e: - logger.error(f"Error accessing entry: {entry_path}: {e}") + except PermissionError as e: + # Skip files we don't have permission to access + logger.error(f"Permission denied accessing entry: {entry_path}: {e}") continue diff --git a/tests/test_filestore.py b/tests/test_filestore.py index 4cda813f..c5c6e623 100644 --- a/tests/test_filestore.py +++ b/tests/test_filestore.py @@ -186,10 +186,9 @@ def test_symlink_detection(test_dir): symlink_path = os.path.join(test_dir, "link_to_target") os.symlink(target_file, symlink_path) - # Get FileInfo using stat (follow symlink for stat_result) - # Note: pass the symlink path as absolute_path so lstat can detect it + lstat_result = os.lstat(symlink_path) stat_result = os.stat(symlink_path) - file_info = FileInfo.from_stat("link_to_target", symlink_path, stat_result) + file_info = FileInfo.from_stat("link_to_target", symlink_path, lstat_result, stat_result) assert file_info.is_symlink is True assert file_info.name == "link_to_target" @@ -364,8 +363,8 @@ def test_yield_file_infos_with_symlinks(filestore, test_dir): database.find_fsp_from_absolute_path = original_find -def test_broken_symlink_not_listed(filestore, test_dir): - """Test that broken symlinks are not listed (caught by OSError)""" +def test_broken_symlink_is_listed(filestore, test_dir): + """Test that broken symlinks are listed with is_symlink=True and symlink_target_fsp=None""" # Create a broken symlink broken_link = os.path.join(test_dir, "broken_link") os.symlink("/nonexistent/path", broken_link) @@ -374,12 +373,18 @@ def test_broken_symlink_not_listed(filestore, test_dir): with open(os.path.join(test_dir, "valid_file.txt"), "w") as f: f.write("valid") - # List directory - broken symlink should not appear + # List directory - broken symlink should now appear files = list(filestore.yield_file_infos("")) file_names = [f.name for f in files] assert "valid_file.txt" in file_names - assert "broken_link" not in file_names # Broken symlink should be skipped + assert "broken_link" in file_names # Broken symlink is now listed + + # Find the broken symlink and verify its properties + broken_link_info = next((f for f in files if f.name == "broken_link"), None) + assert broken_link_info is not None + assert broken_link_info.is_symlink is True + assert broken_link_info.symlink_target_fsp is None # Target not resolvable def test_symlink_to_directory(filestore, test_dir): @@ -413,3 +418,20 @@ def test_symlink_to_directory(filestore, test_dir): assert symlink_info.symlink_target_fsp is not None finally: database.find_fsp_from_absolute_path = original_find + + +def test_broken_symlink_detection(test_dir, filestore): + """Test that broken symlinks are detected and returned with is_symlink=True""" + # Create a broken symlink + broken_link_path = os.path.join(test_dir, "broken_link") + os.symlink("/nonexistent/path", broken_link_path) + + # Get file infos - broken symlink should be included + file_infos = list(filestore.yield_file_infos(None)) + + # Find the broken symlink in results + broken_link_info = next((f for f in file_infos if f.name == "broken_link"), None) + + assert broken_link_info is not None, "Broken symlink should be returned" + assert broken_link_info.is_symlink is True, "Should be marked as symlink" + assert broken_link_info.symlink_target_fsp is None, "Target should be None for broken symlink" From f795647747f09bca5cce1c864014ccdd5fc3dde1 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Fri, 6 Feb 2026 21:06:51 +0000 Subject: [PATCH 18/39] feat: display broken symlinks with broken link icon and error styling --- .../components/ui/BrowsePage/FileTable.tsx | 51 ++++++++++++------- .../ui/BrowsePage/fileTableColumns.tsx | 1 + frontend/src/contexts/FileBrowserContext.tsx | 7 ++- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/frontend/src/components/ui/BrowsePage/FileTable.tsx b/frontend/src/components/ui/BrowsePage/FileTable.tsx index f972bcba..1ea11844 100644 --- a/frontend/src/components/ui/BrowsePage/FileTable.tsx +++ b/frontend/src/components/ui/BrowsePage/FileTable.tsx @@ -9,7 +9,7 @@ import { type SortingState } from '@tanstack/react-table'; import { IconButton, Typography } from '@material-tailwind/react'; -import { TbFile, TbLink } from 'react-icons/tb'; +import { TbFile, TbLink, TbLinkOff } from 'react-icons/tb'; import { HiOutlineEllipsisHorizontalCircle, HiOutlineFolder @@ -59,13 +59,17 @@ export default function Table({ const file = row.original; const name = getValue() as string; let link = '#'; + let isBrokenSymlink = false; if (file.is_symlink && file.symlink_target_fsp) { - // Symlink with known target in a valid file share + // Valid symlink with known target in a valid file share link = makeBrowseLink( file.symlink_target_fsp.fsp_name, file.symlink_target_fsp.subpath ) as string; + } else if (file.is_symlink && !file.symlink_target_fsp) { + // Broken symlink - no valid target + isBrokenSymlink = true; } else if (file.is_dir && fileQuery.data?.currentFileSharePath) { // Regular directory link = makeBrowseLink( @@ -76,7 +80,9 @@ export default function Table({ return (
- {file.is_symlink ? ( + {isBrokenSymlink ? ( + + ) : file.is_symlink ? ( ) : file.is_dir ? ( @@ -84,7 +90,11 @@ export default function Table({ )} - {file.is_dir || file.is_symlink ? ( + {isBrokenSymlink ? ( + + {name} + + ) : file.is_dir || file.is_symlink ? ( {name} @@ -103,20 +113,25 @@ export default function Table({ { id: 'actions', header: 'Actions', - cell: ({ row }) => ( -
- { - e.stopPropagation(); - handleContextMenuClick(e as any, row.original); - }} - variant="ghost" - > - - -
- ), + cell: ({ row }) => { + const file = row.original; + const isBrokenSymlink = file.is_symlink && !file.symlink_target_fsp; + return ( +
+ { + e.stopPropagation(); + handleContextMenuClick(e as any, row.original); + }} + variant="ghost" + > + + +
+ ); + }, size: 30, enableSorting: false } diff --git a/frontend/src/components/ui/BrowsePage/fileTableColumns.tsx b/frontend/src/components/ui/BrowsePage/fileTableColumns.tsx index 7ef13884..65e39e99 100644 --- a/frontend/src/components/ui/BrowsePage/fileTableColumns.tsx +++ b/frontend/src/components/ui/BrowsePage/fileTableColumns.tsx @@ -13,6 +13,7 @@ export const typeColumn: ColumnDef = { cell: ({ row }) => { const file = row.original; if (file.is_symlink) { + // Valid symlink return Symlink; } return {file.is_dir ? 'Folder' : 'File'}; diff --git a/frontend/src/contexts/FileBrowserContext.tsx b/frontend/src/contexts/FileBrowserContext.tsx index 7185a4f9..86c921fc 100644 --- a/frontend/src/contexts/FileBrowserContext.tsx +++ b/frontend/src/contexts/FileBrowserContext.tsx @@ -133,8 +133,13 @@ export const FileBrowserContextProvider = ({ file: FileOrFolder, showFilePropertiesDrawer: boolean ) => { + const isBrokenSymlink = file.is_symlink && !file.symlink_target_fsp; // If clicking on a file (not directory), navigate to the file URL - if (!file.is_dir && fileQuery.data?.currentFileSharePath) { + if ( + !file.is_dir && + fileQuery.data?.currentFileSharePath && + !isBrokenSymlink + ) { const fileLink = makeBrowseLink( fileQuery.data?.currentFileSharePath.name, file.path From 6ae3968a98bf5a23b122b1642da7b07646b91a89 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Fri, 6 Feb 2026 21:09:10 +0000 Subject: [PATCH 19/39] test: update E2E test to verify broken symlinks are displayed with broken link icon --- .../ui-tests/tests/symlink-navigation.spec.ts | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/frontend/ui-tests/tests/symlink-navigation.spec.ts b/frontend/ui-tests/tests/symlink-navigation.spec.ts index 072b5565..a440186f 100644 --- a/frontend/ui-tests/tests/symlink-navigation.spec.ts +++ b/frontend/ui-tests/tests/symlink-navigation.spec.ts @@ -199,17 +199,35 @@ test.describe('Symlink Navigation and Display', () => { await page.keyboard.press('Escape'); }); - test('broken symlinks are not displayed', async ({ + test('broken symlinks are displayed with broken link icon', async ({ fileglancerPage: page }) => { // The broken_symlink was created pointing to /nonexistent/path - // It should NOT appear in the file list because os.stat() fails on broken symlinks - // and the backend catches that error and skips the entry + // It should now appear in the file list with a broken link icon + const brokenLinkRow = page + .getByRole('row') + .filter({ hasText: 'broken_symlink' }); + + await expect(brokenLinkRow).toBeVisible(); + + // Verify broken link icon is visible (TbLinkOff with text-error class) + await expect(brokenLinkRow.locator('svg.text-error')).toBeVisible(); + + // Verify Type column shows "Symlink" await expect( - page.getByText('broken_symlink', { exact: true }) - ).not.toBeVisible(); + brokenLinkRow.getByText('Symlink', { exact: true }) + ).toBeVisible(); + + // Verify the name is NOT a hyperlink (should be plain Typography) + // Get the text element and verify it's not an anchor + const nameCell = brokenLinkRow.locator('td').first(); + const linkElement = nameCell.locator('a'); + await expect(linkElement).not.toBeVisible(); + + // But the text itself should be visible + await expect(nameCell.getByText('broken_symlink')).toBeVisible(); - // But valid files should still be visible + // Verify valid files are still visible await expect( page.getByText('regular_file.txt', { exact: true }) ).toBeVisible(); From 4a9981e759e451a11a4fb2b983765b2261f1fe28 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Fri, 6 Feb 2026 21:10:00 +0000 Subject: [PATCH 20/39] test: add integration test for broken symlink API response --- tests/test_endpoints.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/test_endpoints.py b/tests/test_endpoints.py index 179f6de3..16c512a2 100644 --- a/tests/test_endpoints.py +++ b/tests/test_endpoints.py @@ -1188,3 +1188,33 @@ def test_head_content_with_symlink(test_client, temp_dir): assert response.headers["Accept-Ranges"] == "bytes" assert "Content-Length" in response.headers assert int(response.headers["Content-Length"]) == len(target_content) + + +def test_broken_symlink_in_file_listing(test_client, temp_dir): + """Test that broken symlinks appear in /api/files response with correct properties""" + # Create a broken symlink pointing to a nonexistent path + broken_link_path = os.path.join(temp_dir, "broken_link") + os.symlink("/nonexistent/path", broken_link_path) + + # Create a regular file for comparison + regular_file = os.path.join(temp_dir, "regular.txt") + with open(regular_file, "w") as f: + f.write("content") + + # Request file listing + response = test_client.get("/api/files/tempdir") + assert response.status_code == 200 + + data = response.json() + files = data["files"] + + # Find broken symlink in response + broken_link_file = next((f for f in files if f["name"] == "broken_link"), None) + assert broken_link_file is not None, "Broken symlink should be in response" + assert broken_link_file["is_symlink"] is True, "Should be marked as symlink" + assert broken_link_file["symlink_target_fsp"] is None, "Target should be None for broken symlink" + + # Regular file should also be present + regular = next((f for f in files if f["name"] == "regular.txt"), None) + assert regular is not None, "Regular file should be in response" + assert regular["is_symlink"] is False, "Regular file should not be marked as symlink" From 7d000cf53c8b27f3092e163645af7e0b966b46e9 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Mon, 9 Feb 2026 15:55:02 +0000 Subject: [PATCH 21/39] refactor: inline _get_stat_result into _get_file_info_from_path Keep all filesystem I/O in the same method as the path validation so CodeQL can trace the sanitizer to the os.stat calls. --- fileglancer/filestore.py | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index 9834f598..c7efb0e7 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -73,26 +73,6 @@ def _safe_readlink(path: str, root_path: Optional[str] = None) -> Optional[str]: logger.warning(f"Failed to read symlink target for {path}: {e}") return None - @staticmethod - def _get_stat_result(absolute_path: str, is_symlink: bool, lstat_result: os.stat_result) -> os.stat_result: - """ - Get the appropriate stat result for a file, handling broken symlinks. - - For symlinks, attempts to stat the target. If that fails (broken symlink), - returns the lstat result instead. - """ - if is_symlink: - try: - # Try to stat the target for file metadata - return os.stat(absolute_path) - except (FileNotFoundError, PermissionError, OSError) as e: - # Broken symlink - use lstat_result as the stat_result - logger.warning(f"Broken symlink detected: {absolute_path}: {e}") - return lstat_result - else: - # Not a symlink, stat it normally - return os.stat(absolute_path) - @classmethod def _get_symlink_target_fsp(cls, absolute_path: str, is_symlink: bool, session, root_path: Optional[str]) -> Optional[dict]: @@ -307,6 +287,11 @@ def _get_file_info_from_path(self, full_path: str, current_user: str = None, ses components. We pass full_path (not realpath) to from_stat so that lstat() can detect symlinks. Symlink targets may be outside the root (cross-fileshare symlinks), which is valid - we detect and report them without following. + + All filesystem I/O (lstat/stat) is performed here rather than in + FileInfo.from_stat so that path validation and I/O are in the same + method, which allows static analysis tools (CodeQL) to see that the + path is sanitized before use. """ root_real = os.path.realpath(self.root_path) @@ -343,11 +328,17 @@ def _is_within_root(p: str) -> bool: else: rel_path = os.path.relpath(full_real, root_real) - # Perform filesystem stat calls here, after validation, so that - # the sanitizer and the I/O are in the same method. + # Perform all filesystem stat calls here, after validation. lstat_result = os.lstat(full_path) is_symlink = stat.S_ISLNK(lstat_result.st_mode) - stat_result = FileInfo._get_stat_result(full_path, is_symlink, lstat_result) + if is_symlink: + try: + stat_result = os.stat(full_path) + except (FileNotFoundError, PermissionError, OSError) as e: + logger.warning(f"Broken symlink detected: {full_path}: {e}") + stat_result = lstat_result + else: + stat_result = os.stat(full_path) return FileInfo.from_stat( rel_path, full_path, lstat_result, stat_result, From 08fff7faa182b66906c334b95c82b2e376f6f6b5 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Mon, 9 Feb 2026 11:37:29 -0500 Subject: [PATCH 22/39] test: add integration tests for symlink traversal through directory and double-hop symlinks --- tests/test_endpoints.py | 56 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/test_endpoints.py b/tests/test_endpoints.py index 16c512a2..1b3bd0f3 100644 --- a/tests/test_endpoints.py +++ b/tests/test_endpoints.py @@ -1169,6 +1169,62 @@ def test_get_content_with_symlink_no_matching_fsp(test_client, temp_dir): shutil.rmtree(external_dir) +def test_get_content_traversal_through_directory_symlink(test_client, temp_dir): + """Test that accessing a file through a directory symlink pointing outside the FSP is blocked""" + external_dir = tempfile.mkdtemp() + + try: + # Create a file in the external directory + external_file = os.path.join(external_dir, "secret.txt") + with open(external_file, "w") as f: + f.write("sensitive data") + + # Create a directory symlink in the FSP pointing to the external directory + symlink_path = os.path.join(temp_dir, "link_to_external_dir") + os.symlink(external_dir, symlink_path) + + # Try to access a file *through* the directory symlink + response = test_client.get( + "/api/content/tempdir?subpath=link_to_external_dir/secret.txt" + ) + # Should be blocked — the resolved path escapes the FSP root + assert response.status_code == 400 + + finally: + shutil.rmtree(external_dir) + + +def test_get_content_double_hop_symlink(test_client, temp_dir): + """Test that accessing a file through a chain of symlinks pointing outside the FSP is blocked""" + external_dir = tempfile.mkdtemp() + hop_dir = tempfile.mkdtemp() + + try: + # Create a file in the external directory + external_file = os.path.join(external_dir, "secret.txt") + with open(external_file, "w") as f: + f.write("sensitive data") + + # Create first hop: hop_dir/hop2 -> external_dir + hop2_path = os.path.join(hop_dir, "hop2") + os.symlink(external_dir, hop2_path) + + # Create second hop in the FSP: double_hop -> hop_dir/hop2 + symlink_path = os.path.join(temp_dir, "double_hop") + os.symlink(hop2_path, symlink_path) + + # Try to access a file through the double-hop symlink + response = test_client.get( + "/api/content/tempdir?subpath=double_hop/secret.txt" + ) + # Should be blocked — the resolved path escapes the FSP root + assert response.status_code == 400 + + finally: + shutil.rmtree(external_dir) + shutil.rmtree(hop_dir) + + def test_head_content_with_symlink(test_client, temp_dir): """Test HEAD request to /api/content endpoint with a symlink""" # Create a target file within the FSP From 733e318381d49f649dcf248acdd09ced5a91e977 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Mon, 9 Feb 2026 12:51:29 -0500 Subject: [PATCH 23/39] fix: move import statement to top of file --- fileglancer/filestore.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index c7efb0e7..24b44872 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -14,6 +14,7 @@ from loguru import logger from .model import FileSharePath +from .utils import is_likely_binary # Default buffer size for streaming file contents DEFAULT_BUFFER_SIZE = 8192 @@ -410,8 +411,6 @@ def check_is_binary(self, path: Optional[str] = None, sample_size: int = 4096) - FileNotFoundError: If the file does not exist PermissionError: If the file cannot be read """ - from .utils import is_likely_binary - full_path = self._check_path_in_root(path) # Directories are not binary From 08f599e562d7ff444ed51d69fa4d5b0dea5c16f5 Mon Sep 17 00:00:00 2001 From: Jody Clements Date: Tue, 10 Feb 2026 10:10:25 -0500 Subject: [PATCH 24/39] fix: Symbolic link pointing to non-existent file now show as broken. - Adds os.path.exists(target) check before attempting file share resolution - Returns None immediately for broken symlinks - Valid symlinks proceed to normal resolution logic --- fileglancer/filestore.py | 6 +++++- tests/test_filestore.py | 45 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index 24b44872..84f5e196 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -96,8 +96,12 @@ def _get_symlink_target_fsp(cls, absolute_path: str, is_symlink: bool, session, target = os.path.join(os.path.dirname(absolute_path), target) target = os.path.abspath(target) + # Check if the symlink target actually exists + # If it doesn't exist, return None (broken symlink) + if not os.path.exists(target): + return None + # Try to find which file share contains this target - # If stat failed earlier (broken symlink), we won't find a match try: from fileglancer.database import find_fsp_from_absolute_path match = find_fsp_from_absolute_path(session, target) diff --git a/tests/test_filestore.py b/tests/test_filestore.py index c5c6e623..4caa25d0 100644 --- a/tests/test_filestore.py +++ b/tests/test_filestore.py @@ -435,3 +435,48 @@ def test_broken_symlink_detection(test_dir, filestore): assert broken_link_info is not None, "Broken symlink should be returned" assert broken_link_info.is_symlink is True, "Should be marked as symlink" assert broken_link_info.symlink_target_fsp is None, "Target should be None for broken symlink" + + +def test_broken_symlink_within_share(test_dir): + """Test that broken symlinks pointing to paths within a file share don't get symlink_target_fsp populated""" + # Create a filestore + fsp = FileSharePath(zone="test", name="test_share", mount_path=test_dir) + filestore = Filestore(fsp) + + # Create a broken symlink that points to a non-existent file within the share + broken_link_path = os.path.join(test_dir, "link_to_missing_file") + missing_target = os.path.join(test_dir, "subdir", "nonexistent.txt") + os.symlink(missing_target, broken_link_path) + + # Mock the database session and find_fsp_from_absolute_path + from unittest.mock import Mock + from fileglancer import database + + mock_session = Mock() + original_find = database.find_fsp_from_absolute_path + + def mock_find(session, path): + # This would normally match the file share pattern, + # but since the target doesn't exist, we should not get here + # because os.path.exists check should return False first + normalized_path = os.path.realpath(path) + test_dir_real = os.path.realpath(test_dir) + if normalized_path.startswith(test_dir_real): + return (fsp, os.path.relpath(normalized_path, test_dir_real)) + return None + + database.find_fsp_from_absolute_path = mock_find + + try: + # Get file infos with session (so symlink resolution is attempted) + file_infos = list(filestore.yield_file_infos("", session=mock_session)) + + # Find the broken symlink + broken_link_info = next((f for f in file_infos if f.name == "link_to_missing_file"), None) + + # Verify the symlink is detected but target is not resolved + assert broken_link_info is not None, "Broken symlink should be listed" + assert broken_link_info.is_symlink is True, "Should be marked as symlink" + assert broken_link_info.symlink_target_fsp is None, "symlink_target_fsp should be None for broken symlink even if target path matches share pattern" + finally: + database.find_fsp_from_absolute_path = original_find From d21adcf8c88b4c426e910fd370b35d8b73e76ab0 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Tue, 10 Feb 2026 11:18:43 -0500 Subject: [PATCH 25/39] wip: attempt to satisfy the CodeQL security warning --- fileglancer/filestore.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index 84f5e196..a4ba913e 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -13,6 +13,7 @@ from typing import Optional, Generator from loguru import logger +from .database import find_fsp_from_absolute_path from .model import FileSharePath from .utils import is_likely_binary @@ -96,16 +97,14 @@ def _get_symlink_target_fsp(cls, absolute_path: str, is_symlink: bool, session, target = os.path.join(os.path.dirname(absolute_path), target) target = os.path.abspath(target) - # Check if the symlink target actually exists - # If it doesn't exist, return None (broken symlink) - if not os.path.exists(target): - return None - # Try to find which file share contains this target - try: - from fileglancer.database import find_fsp_from_absolute_path + try: match = find_fsp_from_absolute_path(session, target) if match: + # Check if the symlink target actually exists + # If it doesn't exist, return None (broken symlink) + if not os.path.exists(target): + return None fsp, subpath = match return {"fsp_name": fsp.name, "subpath": subpath} except (FileNotFoundError, PermissionError, OSError): From fc6cc50b76213b1fbd22ee4635a4e47c886043a7 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Tue, 10 Feb 2026 11:27:09 -0500 Subject: [PATCH 26/39] Potential fix for code scanning alert no. 69: Uncontrolled data used in path expression Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- fileglancer/filestore.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index a4ba913e..d6576d0a 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -98,14 +98,23 @@ def _get_symlink_target_fsp(cls, absolute_path: str, is_symlink: bool, session, target = os.path.abspath(target) # Try to find which file share contains this target - try: + try: match = find_fsp_from_absolute_path(session, target) if match: - # Check if the symlink target actually exists + fsp, subpath = match + + # Reconstruct the canonical target path from the file share root + # and the returned subpath so we only operate within managed roots. + if subpath: + validated_target = os.path.realpath(os.path.join(fsp.path, subpath)) + else: + validated_target = os.path.realpath(fsp.path) + + # Check if the symlink target actually exists within the resolved location. # If it doesn't exist, return None (broken symlink) - if not os.path.exists(target): + if not os.path.exists(validated_target): return None - fsp, subpath = match + return {"fsp_name": fsp.name, "subpath": subpath} except (FileNotFoundError, PermissionError, OSError): # Target doesn't exist or isn't accessible From 738e63b638c7fd5efac316438537afa1989e97f5 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Tue, 10 Feb 2026 14:38:54 -0500 Subject: [PATCH 27/39] fix: correct property on fsp from path to mount_path --- fileglancer/filestore.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index d6576d0a..6c3d42a4 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -106,9 +106,9 @@ def _get_symlink_target_fsp(cls, absolute_path: str, is_symlink: bool, session, # Reconstruct the canonical target path from the file share root # and the returned subpath so we only operate within managed roots. if subpath: - validated_target = os.path.realpath(os.path.join(fsp.path, subpath)) + validated_target = os.path.realpath(os.path.join(fsp.mount_path, subpath)) else: - validated_target = os.path.realpath(fsp.path) + validated_target = os.path.realpath(fsp.mount_path) # Check if the symlink target actually exists within the resolved location. # If it doesn't exist, return None (broken symlink) From 7aef4f368980ba7733fd7ec62feb55f3a14ffa39 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Tue, 10 Feb 2026 14:39:12 -0500 Subject: [PATCH 28/39] fix: mock database for filestore symlink tests --- tests/test_filestore.py | 111 +++++++++++++++++----------------------- 1 file changed, 47 insertions(+), 64 deletions(-) diff --git a/tests/test_filestore.py b/tests/test_filestore.py index 4caa25d0..f436d0f5 100644 --- a/tests/test_filestore.py +++ b/tests/test_filestore.py @@ -3,6 +3,12 @@ import pytest import tempfile import shutil + +from contextlib import contextmanager +from unittest.mock import Mock +from typing import List, Callable, Optional + +from fileglancer import database from fileglancer.filestore import Filestore, FileInfo from fileglancer.model import FileSharePath @@ -38,6 +44,36 @@ def filestore(test_dir): return Filestore(file_share_path) +@contextmanager +def mock_database_for_symlinks(file_share_paths: List[FileSharePath], + find_fsp_func: Optional[Callable] = None): + """ + Context manager to mock database functions for symlink resolution tests. + + Args: + file_share_paths: List of FileSharePath objects to return from get_file_share_paths + find_fsp_func: Optional custom function for find_fsp_from_absolute_path. + If None, the original function is preserved (not mocked). + """ + original_get_paths = database.get_file_share_paths + original_find = database.find_fsp_from_absolute_path + + def mock_get_file_share_paths(session, fsp_name=None): + paths = file_share_paths + if fsp_name: + paths = [p for p in paths if p.name == fsp_name] + return paths + + try: + database.get_file_share_paths = mock_get_file_share_paths + if find_fsp_func is not None: + database.find_fsp_from_absolute_path = find_fsp_func + yield + finally: + database.get_file_share_paths = original_get_paths + database.find_fsp_from_absolute_path = original_find + + def test_unmounted_filestore(): test_dir = "/not/a/real/path" file_share_path = FileSharePath(zone="test", name="test", mount_path=test_dir) @@ -205,23 +241,16 @@ def test_same_share_symlink_resolution_via_listing(filestore, test_dir): symlink_path = os.path.join(test_dir, "link_to_subdir_file") os.symlink(target_file, symlink_path) - # Get file info via directory listing with session (mock database session) - from unittest.mock import Mock - from fileglancer import database - mock_session = Mock() - original_find = database.find_fsp_from_absolute_path + fsp = FileSharePath(zone="test", name="test", mount_path=test_dir) def mock_find(session, path): # Normalize paths for comparison if os.path.realpath(path) == os.path.realpath(target_file): - fsp = FileSharePath(zone="test", name="test", mount_path=test_dir) return (fsp, "subdir/target_same_share.txt") return None - database.find_fsp_from_absolute_path = mock_find - - try: + with mock_database_for_symlinks([fsp], mock_find): # Use yield_file_infos to list directory - symlinks are detected this way files = list(filestore.yield_file_infos("", session=mock_session)) symlink_info = next((f for f in files if f.name == "link_to_subdir_file"), None) @@ -231,8 +260,6 @@ def mock_find(session, path): assert symlink_info.symlink_target_fsp is not None assert symlink_info.symlink_target_fsp["fsp_name"] == "test" assert symlink_info.symlink_target_fsp["subpath"] == "subdir/target_same_share.txt" - finally: - database.find_fsp_from_absolute_path = original_find def test_cross_share_symlink_resolution_via_listing(test_dir): @@ -254,25 +281,18 @@ def test_cross_share_symlink_resolution_via_listing(test_dir): # Create filestore for share1 fsp1 = FileSharePath(zone="test", name="share1", mount_path=share1_dir) + fsp2 = FileSharePath(zone="test", name="share2", mount_path=share2_dir) filestore1 = Filestore(fsp1) - # Mock database session and find function - from unittest.mock import Mock - from fileglancer import database - mock_session = Mock() - original_find = database.find_fsp_from_absolute_path def mock_find(session, path): # Normalize paths for comparison if os.path.realpath(path) == os.path.realpath(target_file): - fsp2 = FileSharePath(zone="test", name="share2", mount_path=share2_dir) return (fsp2, "target.txt") return None - database.find_fsp_from_absolute_path = mock_find - - try: + with mock_database_for_symlinks([fsp1, fsp2], mock_find): # Use yield_file_infos to list directory - symlinks are detected this way files = list(filestore1.yield_file_infos("", session=mock_session)) symlink_info = next((f for f in files if f.name == "link_to_share2"), None) @@ -282,8 +302,6 @@ def mock_find(session, path): assert symlink_info.symlink_target_fsp is not None assert symlink_info.symlink_target_fsp["fsp_name"] == "share2" assert symlink_info.symlink_target_fsp["subpath"] == "target.txt" - finally: - database.find_fsp_from_absolute_path = original_find def test_relative_symlink_resolution(test_dir): @@ -301,24 +319,18 @@ def test_relative_symlink_resolution(test_dir): # Create filestore for nested_dir so symlink is listed via yield_file_infos fsp = FileSharePath(zone="test", name="nested", mount_path=nested_dir) + fsp_rel = FileSharePath(zone="test", name="rel_test", mount_path=os.path.join(test_dir, "rel_test")) nested_filestore = Filestore(fsp) - from unittest.mock import Mock - from fileglancer import database - mock_session = Mock() - original_find = database.find_fsp_from_absolute_path def mock_find(session, path): if os.path.realpath(path) == os.path.realpath(target_file): # Return fsp for rel_test directory - fsp_rel = FileSharePath(zone="test", name="rel_test", mount_path=os.path.join(test_dir, "rel_test")) return (fsp_rel, "target.txt") return None - database.find_fsp_from_absolute_path = mock_find - - try: + with mock_database_for_symlinks([fsp, fsp_rel], mock_find): # List directory to find the symlink files = list(nested_filestore.yield_file_infos("", session=mock_session)) symlink_info = next((f for f in files if f.name == "link"), None) @@ -327,8 +339,6 @@ def mock_find(session, path): assert symlink_info.is_symlink is True assert symlink_info.symlink_target_fsp is not None assert symlink_info.symlink_target_fsp["subpath"] == "target.txt" - finally: - database.find_fsp_from_absolute_path = original_find def test_yield_file_infos_with_symlinks(filestore, test_dir): @@ -342,25 +352,16 @@ def test_yield_file_infos_with_symlinks(filestore, test_dir): os.path.join(test_dir, "link1") ) - from unittest.mock import Mock - from fileglancer import database - mock_session = Mock() - original_find = database.find_fsp_from_absolute_path - database.find_fsp_from_absolute_path = lambda s, p: ( - FileSharePath(zone="test", name="test", mount_path=test_dir), - "file1.txt" - ) + fsp = FileSharePath(zone="test", name="test", mount_path=test_dir) - try: + with mock_database_for_symlinks([fsp], lambda s, p: (fsp, "file1.txt")): files = list(filestore.yield_file_infos("", session=mock_session)) # Find the symlink in the list symlink_info = next((f for f in files if f.name == "link1"), None) assert symlink_info is not None assert symlink_info.is_symlink is True - finally: - database.find_fsp_from_absolute_path = original_find def test_broken_symlink_is_listed(filestore, test_dir): @@ -397,17 +398,10 @@ def test_symlink_to_directory(filestore, test_dir): symlink_path = os.path.join(test_dir, "link_to_dir") os.symlink(target_dir, symlink_path) - from unittest.mock import Mock - from fileglancer import database - mock_session = Mock() - original_find = database.find_fsp_from_absolute_path - database.find_fsp_from_absolute_path = lambda s, p: ( - FileSharePath(zone="test", name="test", mount_path=test_dir), - "target_dir" - ) + fsp = FileSharePath(zone="test", name="test", mount_path=test_dir) - try: + with mock_database_for_symlinks([fsp], lambda s, p: (fsp, "target_dir")): # Use yield_file_infos to list directory - symlinks to dirs are detected this way files = list(filestore.yield_file_infos("", session=mock_session)) symlink_info = next((f for f in files if f.name == "link_to_dir"), None) @@ -416,8 +410,6 @@ def test_symlink_to_directory(filestore, test_dir): assert symlink_info.is_symlink is True assert symlink_info.is_dir is True # Should also be marked as directory assert symlink_info.symlink_target_fsp is not None - finally: - database.find_fsp_from_absolute_path = original_find def test_broken_symlink_detection(test_dir, filestore): @@ -448,12 +440,7 @@ def test_broken_symlink_within_share(test_dir): missing_target = os.path.join(test_dir, "subdir", "nonexistent.txt") os.symlink(missing_target, broken_link_path) - # Mock the database session and find_fsp_from_absolute_path - from unittest.mock import Mock - from fileglancer import database - mock_session = Mock() - original_find = database.find_fsp_from_absolute_path def mock_find(session, path): # This would normally match the file share pattern, @@ -465,9 +452,7 @@ def mock_find(session, path): return (fsp, os.path.relpath(normalized_path, test_dir_real)) return None - database.find_fsp_from_absolute_path = mock_find - - try: + with mock_database_for_symlinks([fsp], mock_find): # Get file infos with session (so symlink resolution is attempted) file_infos = list(filestore.yield_file_infos("", session=mock_session)) @@ -478,5 +463,3 @@ def mock_find(session, path): assert broken_link_info is not None, "Broken symlink should be listed" assert broken_link_info.is_symlink is True, "Should be marked as symlink" assert broken_link_info.symlink_target_fsp is None, "symlink_target_fsp should be None for broken symlink even if target path matches share pattern" - finally: - database.find_fsp_from_absolute_path = original_find From f03d5512f314447bc9022fd151937e9ed2df5371 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Tue, 10 Feb 2026 15:25:04 -0500 Subject: [PATCH 29/39] fix: action menu options for symlink entries --- .../src/components/ui/BrowsePage/FileBrowser.tsx | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/frontend/src/components/ui/BrowsePage/FileBrowser.tsx b/frontend/src/components/ui/BrowsePage/FileBrowser.tsx index 8f298f50..ab0e24b8 100644 --- a/frontend/src/components/ui/BrowsePage/FileBrowser.tsx +++ b/frontend/src/components/ui/BrowsePage/FileBrowser.tsx @@ -122,7 +122,9 @@ export default function FileBrowser({ toast.error(`Error downloading file: ${result.error}`); } }, - shouldShow: !propertiesTarget.is_dir + shouldShow: + !fileBrowserState.selectedFiles[0]?.is_dir && + !fileBrowserState.selectedFiles[0]?.is_symlink }, { name: isFavorite ? 'Unset favorite' : 'Set favorite', @@ -135,9 +137,8 @@ export default function FileBrowser({ } }, shouldShow: - (fileBrowserState.selectedFiles[0]?.is_dir && - !fileBrowserState.selectedFiles[0].is_symlink) ?? - false + fileBrowserState.selectedFiles[0]?.is_dir && + !fileBrowserState.selectedFiles[0].is_symlink }, { name: 'Convert images to OME-Zarr', @@ -146,8 +147,8 @@ export default function FileBrowser({ }, shouldShow: tasksEnabled && - propertiesTarget.is_dir && - !propertiesTarget.is_symlink + fileBrowserState.selectedFiles[0]?.is_dir && + !fileBrowserState.selectedFiles[0]?.is_symlink }, { name: 'Rename', @@ -161,7 +162,7 @@ export default function FileBrowser({ action: () => { setShowPermissionsDialog(true); }, - shouldShow: !propertiesTarget.is_dir + shouldShow: true }, { name: 'Delete', From 7daed3532bce7e3063a91a6173cdcd1c4bd51d53 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Tue, 10 Feb 2026 16:34:46 -0500 Subject: [PATCH 30/39] fix: resolve properties target by name to avoid symlink path collision --- frontend/src/contexts/FileBrowserContext.tsx | 45 +++++++++++++++----- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/frontend/src/contexts/FileBrowserContext.tsx b/frontend/src/contexts/FileBrowserContext.tsx index 86c921fc..39167085 100644 --- a/frontend/src/contexts/FileBrowserContext.tsx +++ b/frontend/src/contexts/FileBrowserContext.tsx @@ -35,6 +35,7 @@ type FileBrowserState = { // Internal state type InternalFileBrowserState = { propertiesTargetPath: string | null; // Store path instead of full object + propertiesTargetName: string | null; // Store name for unique lookup in file listings selectedFiles: FileOrFolder[]; }; @@ -104,6 +105,7 @@ export const FileBrowserContextProvider = ({ // Internal state for UI interactions const [internalState, setInternalState] = useState({ propertiesTargetPath: null, + propertiesTargetName: null, selectedFiles: [] }); @@ -156,15 +158,14 @@ export const FileBrowserContextProvider = ({ showFilePropertiesDrawer ? [file] : []; - const newPropertiesTargetPath = + const isSelected = currentIndex === -1 || internalState.selectedFiles.length > 1 || - showFilePropertiesDrawer - ? file.path - : null; + showFilePropertiesDrawer; updateInternalState({ - propertiesTargetPath: newPropertiesTargetPath, + propertiesTargetPath: isSelected ? file.path : null, + propertiesTargetName: isSelected ? file.name : null, selectedFiles: newSelectedFiles }); }; @@ -176,6 +177,7 @@ export const FileBrowserContextProvider = ({ updateInternalState({ propertiesTargetPath: file.path, + propertiesTargetName: file.name, selectedFiles: newSelectedFiles }); }; @@ -190,6 +192,7 @@ export const FileBrowserContextProvider = ({ setInternalState({ propertiesTargetPath: fileQuery.data?.currentFileOrFolder?.path || null, + propertiesTargetName: null, selectedFiles: [] }); } @@ -213,11 +216,15 @@ export const FileBrowserContextProvider = ({ if (renameMutation.isSuccess && renameMutation.variables) { const { oldPath, newPath } = renameMutation.variables; - // If the renamed file was the propertiesTarget, update to the new path + // If the renamed file was the propertiesTarget, update to the new path/name if (internalState.propertiesTargetPath === oldPath) { + const newName = newPath.includes('/') + ? newPath.split('/').pop()! + : newPath; setInternalState(prev => ({ ...prev, - propertiesTargetPath: newPath + propertiesTargetPath: newPath, + propertiesTargetName: newName })); } // Reset mutation state to prevent re-running @@ -230,14 +237,14 @@ export const FileBrowserContextProvider = ({ renameMutation ]); - // Derive propertiesTarget from propertiesTargetPath and fresh query data + // Derive propertiesTarget from propertiesTargetPath/Name and fresh query data // This ensures mutations (rename, permissions) are correctly reflected and don't use a useEffect const propertiesTarget = useMemo(() => { if (!internalState.propertiesTargetPath || !fileQuery.data) { return null; } - // Check if propertiesTargetPath matches the current folder + // Check if propertiesTargetPath matches the current folder (navigation case) if ( fileQuery.data.currentFileOrFolder?.path === internalState.propertiesTargetPath @@ -245,13 +252,29 @@ export const FileBrowserContextProvider = ({ return fileQuery.data.currentFileOrFolder; } - // Otherwise, is it a child of current folder + // Find child by name when available (click selection case). + // Name is unique within a directory and avoids ambiguity from symlinks + // whose resolved path may collide with their target's path. + if (internalState.propertiesTargetName) { + const foundFile = fileQuery.data.files.find( + f => f.name === internalState.propertiesTargetName + ); + if (foundFile) { + return foundFile; + } + } + + // Fallback: find by path const foundFile = fileQuery.data.files.find( f => f.path === internalState.propertiesTargetPath ); return foundFile || null; - }, [internalState.propertiesTargetPath, fileQuery.data]); + }, [ + internalState.propertiesTargetPath, + internalState.propertiesTargetName, + fileQuery.data + ]); return ( Date: Tue, 10 Feb 2026 16:35:00 -0500 Subject: [PATCH 31/39] feat: show symlink icon, name, and target path in properties drawer --- .../ui/PropertiesDrawer/PropertiesDrawer.tsx | 67 ++++++++++++++----- 1 file changed, 50 insertions(+), 17 deletions(-) diff --git a/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx b/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx index 36dbe8f6..76a15c90 100644 --- a/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx +++ b/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx @@ -25,6 +25,7 @@ import { useTicketContext } from '@/contexts/TicketsContext'; import { useProxiedPathContext } from '@/contexts/ProxiedPathContext'; import { useExternalBucketContext } from '@/contexts/ExternalBucketContext'; import useDataToolLinks from '@/hooks/useDataToolLinks'; +import { TbLink, TbLinkOff } from 'react-icons/tb'; type PropertiesDrawerProps = { readonly togglePropertiesDrawer: () => void; @@ -38,17 +39,23 @@ type PropertiesDrawerProps = { function CopyPathButton({ path, - isDataLink + isDataLink, + isSymlink }: { readonly path: string; readonly isDataLink?: boolean; + readonly isSymlink?: boolean; }) { return (
- {isDataLink ? 'Data Link: ' : 'Path: '} + {isDataLink + ? 'Data Link: ' + : isSymlink + ? 'Linked path: ' + : 'Path: '} {path} @@ -60,11 +67,11 @@ function CopyPathButton({ const result = await copyToClipboard(path); if (result.success) { toast.success( - `${isDataLink ? 'Data link' : 'Path'} copied to clipboard!` + `${isDataLink ? 'Data link' : isSymlink ? 'Linked path' : 'Path'} copied to clipboard!` ); } else { toast.error( - `Failed to copy ${isDataLink ? 'data link' : 'path'}. Error: ${result.error}` + `Failed to copy ${isDataLink ? 'data link' : isSymlink ? 'linked path' : 'path'}. Error: ${result.error}` ); } }} @@ -137,21 +144,44 @@ export default function PropertiesDrawer({
- {fileBrowserState.propertiesTarget ? ( + {fileQuery.data?.currentFileOrFolder && + fileBrowserState.propertiesTarget ? (
- {fileBrowserState.propertiesTarget.is_dir ? ( - + {fileBrowserState.propertiesTarget.is_symlink ? ( + <> + {fileBrowserState.propertiesTarget.symlink_target_fsp ? ( + + ) : ( + + )} +
+ + + {fileBrowserState.propertiesTarget.name} + + +
+ ) : ( - + <> + {fileBrowserState.propertiesTarget.is_dir ? ( + + ) : ( + + )} + + + {fileBrowserState.propertiesTarget?.name} + + + )} - - - {fileBrowserState.propertiesTarget?.name} - -
) : ( @@ -196,7 +226,10 @@ export default function PropertiesDrawer({ className="flex-1 flex flex-col gap-4 max-w-full p-2" value="overview" > - + {fileBrowserState.propertiesTarget.is_dir && (proxiedPathByFspAndPathQuery.isPending || From a8c6471e73e5e457d9e794aa1ba63585f53cb0b5 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Tue, 10 Feb 2026 21:48:21 +0000 Subject: [PATCH 32/39] feat: show symlink type and size in overview table --- .../ui/PropertiesDrawer/OverviewTable.tsx | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/ui/PropertiesDrawer/OverviewTable.tsx b/frontend/src/components/ui/PropertiesDrawer/OverviewTable.tsx index 22cdfba5..64937f2b 100644 --- a/frontend/src/components/ui/PropertiesDrawer/OverviewTable.tsx +++ b/frontend/src/components/ui/PropertiesDrawer/OverviewTable.tsx @@ -15,7 +15,15 @@ export default function OverviewTable({ Type - {file ? (file.is_dir ? 'Folder' : 'File') : null} + {file ? ( + file.is_symlink ? ( + file.symlink_target_fsp ? 'Symlink' : 'Symlink (broken)' + ) : file.is_dir ? ( + 'Folder' + ) : ( + 'File' + ) + ) : null} @@ -31,7 +39,9 @@ export default function OverviewTable({ Size - {file ? (file.is_dir ? '—' : formatFileSize(file.size)) : null} + {file ? ( + file.is_dir && !file.is_symlink ? '—' : formatFileSize(file.size) + ) : null} From 2bb7bc3014c8b2b6e598b28332292462ab905489 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Tue, 10 Feb 2026 21:48:27 +0000 Subject: [PATCH 33/39] feat: hide convert tab for symlink entries --- .../src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx b/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx index 76a15c90..f14e8515 100644 --- a/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx +++ b/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx @@ -210,7 +210,7 @@ export default function PropertiesDrawer({ Permissions - {tasksEnabled ? ( + {tasksEnabled && !fileBrowserState.propertiesTarget.is_symlink ? ( {/*Task panel*/} - {tasksEnabled ? ( + {tasksEnabled && !fileBrowserState.propertiesTarget.is_symlink ? ( Date: Wed, 11 Feb 2026 07:36:32 -0500 Subject: [PATCH 34/39] fix: for file symlinks, left click on row should not open file --- frontend/src/contexts/FileBrowserContext.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/frontend/src/contexts/FileBrowserContext.tsx b/frontend/src/contexts/FileBrowserContext.tsx index 39167085..4329e100 100644 --- a/frontend/src/contexts/FileBrowserContext.tsx +++ b/frontend/src/contexts/FileBrowserContext.tsx @@ -135,12 +135,11 @@ export const FileBrowserContextProvider = ({ file: FileOrFolder, showFilePropertiesDrawer: boolean ) => { - const isBrokenSymlink = file.is_symlink && !file.symlink_target_fsp; - // If clicking on a file (not directory), navigate to the file URL + // If clicking on a regular file (not directory or symlink), navigate to the file URL if ( !file.is_dir && - fileQuery.data?.currentFileSharePath && - !isBrokenSymlink + !file.is_symlink && + fileQuery.data?.currentFileSharePath ) { const fileLink = makeBrowseLink( fileQuery.data?.currentFileSharePath.name, From 595e6691c0d929b07f31ba0e3aa02ea1ecb86109 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 11 Feb 2026 07:36:48 -0500 Subject: [PATCH 35/39] tests: symlink display in properties panel --- .../ui-tests/tests/symlink-navigation.spec.ts | 101 +++++++++++++++++- 1 file changed, 100 insertions(+), 1 deletion(-) diff --git a/frontend/ui-tests/tests/symlink-navigation.spec.ts b/frontend/ui-tests/tests/symlink-navigation.spec.ts index a440186f..a663aea4 100644 --- a/frontend/ui-tests/tests/symlink-navigation.spec.ts +++ b/frontend/ui-tests/tests/symlink-navigation.spec.ts @@ -236,7 +236,7 @@ test.describe('Symlink Navigation and Display', () => { ).toBeVisible(); }); - test('clicking a directory symlink row selects it and shows in properties panel', async ({ + test('working directory symlink properties panel shows correct content', async ({ fileglancerPage: page }) => { // Verify properties panel is visible first @@ -255,5 +255,104 @@ test.describe('Symlink Navigation and Display', () => { await expect( propertiesPanel.getByText('symlink_to_subdir', { exact: true }) ).toBeVisible({ timeout: 10000 }); + + // Verify "Linked path:" label (symlink-specific) + await expect(propertiesPanel.getByText('Linked path:')).toBeVisible(); + + // Verify Overview table shows "Symlink" type + await expect( + propertiesPanel.locator('table').getByText('Symlink', { exact: true }) + ).toBeVisible(); + + // Verify Convert tab is not shown + await expect( + propertiesPanel.getByRole('tab', { name: 'Convert' }) + ).not.toBeVisible(); + + // Data link section should be visible (is_dir=true for directory symlink) + await expect(propertiesPanel.getByText(/data link/i).first()).toBeVisible({ + timeout: 10000 + }); + }); + + test('working file symlink properties panel shows correct content', async ({ + fileglancerPage: page + }) => { + const propertiesPanel = page + .locator('[role="complementary"]') + .filter({ hasText: 'Properties' }); + await expect(propertiesPanel).toBeVisible(); + + // Click the Type column to select + const symlinkRow = page + .getByRole('row') + .filter({ hasText: 'symlink_to_file' }); + await symlinkRow.getByText('Symlink', { exact: true }).click(); + + // Wait for properties panel to populate + await expect( + propertiesPanel.getByText('symlink_to_file', { exact: true }) + ).toBeVisible({ timeout: 10000 }); + + // Verify "Linked path:" label (symlink-specific) + await expect(propertiesPanel.getByText('Linked path:')).toBeVisible(); + + // Verify Overview table shows "Symlink" type + await expect( + propertiesPanel.locator('table').getByText('Symlink', { exact: true }) + ).toBeVisible(); + + // Verify Convert tab is not shown + await expect( + propertiesPanel.getByRole('tab', { name: 'Convert' }) + ).not.toBeVisible(); + + // Data link section should NOT be visible (is_dir=false for file symlink) + await expect( + propertiesPanel.getByText('Create data link') + ).not.toBeVisible(); + }); + + test('broken symlink properties panel shows correct content', async ({ + fileglancerPage: page + }) => { + const propertiesPanel = page + .locator('[role="complementary"]') + .filter({ hasText: 'Properties' }); + await expect(propertiesPanel).toBeVisible(); + + // Click the Type column to select + const brokenRow = page + .getByRole('row') + .filter({ hasText: 'broken_symlink' }); + await brokenRow.getByText('Symlink', { exact: true }).click(); + + // Wait for properties panel to populate + await expect( + propertiesPanel.getByText('broken_symlink', { exact: true }) + ).toBeVisible({ timeout: 10000 }); + + // Verify broken link icon (has text-error class) + await expect(propertiesPanel.locator('svg.text-error')).toBeVisible(); + + // Verify "Linked path:" label + await expect(propertiesPanel.getByText('Linked path:')).toBeVisible(); + + // Verify Overview table shows "Symlink (broken)" type + await expect( + propertiesPanel + .locator('table') + .getByText('Symlink (broken)', { exact: true }) + ).toBeVisible(); + + // Verify Convert tab is not shown + await expect( + propertiesPanel.getByRole('tab', { name: 'Convert' }) + ).not.toBeVisible(); + + // Data link section should NOT be visible (is_dir=false for broken symlink) + await expect( + propertiesPanel.getByText('Create data link') + ).not.toBeVisible(); }); }); From 54033972cee8ea29eefa6bc14391aab98de1bf6c Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 11 Feb 2026 07:37:01 -0500 Subject: [PATCH 36/39] chore: prettier formatting --- .../ui/PropertiesDrawer/OverviewTable.tsx | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/frontend/src/components/ui/PropertiesDrawer/OverviewTable.tsx b/frontend/src/components/ui/PropertiesDrawer/OverviewTable.tsx index 64937f2b..f7c09ef0 100644 --- a/frontend/src/components/ui/PropertiesDrawer/OverviewTable.tsx +++ b/frontend/src/components/ui/PropertiesDrawer/OverviewTable.tsx @@ -15,15 +15,15 @@ export default function OverviewTable({ Type - {file ? ( - file.is_symlink ? ( - file.symlink_target_fsp ? 'Symlink' : 'Symlink (broken)' - ) : file.is_dir ? ( - 'Folder' - ) : ( - 'File' - ) - ) : null} + {file + ? file.is_symlink + ? file.symlink_target_fsp + ? 'Symlink' + : 'Symlink (broken)' + : file.is_dir + ? 'Folder' + : 'File' + : null} @@ -39,9 +39,11 @@ export default function OverviewTable({ Size - {file ? ( - file.is_dir && !file.is_symlink ? '—' : formatFileSize(file.size) - ) : null} + {file + ? file.is_dir && !file.is_symlink + ? '—' + : formatFileSize(file.size) + : null} From 5333f6b7843ed945c5a28b1b807088f9544f4da4 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 11 Feb 2026 15:35:59 -0500 Subject: [PATCH 37/39] fix: use properties target path for data link creation instead of current directory - this fixes the case where the user clicks on a subdirectory and then makes a data link from the properties panel --- frontend/src/contexts/FileBrowserContext.tsx | 21 ++++++++++++++- frontend/src/contexts/ProxiedPathContext.tsx | 20 +++++++++++++-- frontend/src/hooks/useDataToolLinks.ts | 27 ++++++++++++-------- frontend/src/hooks/useN5Metadata.ts | 6 ++--- frontend/src/hooks/useZarrMetadata.ts | 6 ++--- 5 files changed, 61 insertions(+), 19 deletions(-) diff --git a/frontend/src/contexts/FileBrowserContext.tsx b/frontend/src/contexts/FileBrowserContext.tsx index 4329e100..ad6b9de9 100644 --- a/frontend/src/contexts/FileBrowserContext.tsx +++ b/frontend/src/contexts/FileBrowserContext.tsx @@ -30,6 +30,7 @@ type FileBrowserContextProviderProps = { type FileBrowserState = { propertiesTarget: FileOrFolder | null; selectedFiles: FileOrFolder[]; + dataLinkPath: string | null; }; // Internal state @@ -275,12 +276,30 @@ export const FileBrowserContextProvider = ({ fileQuery.data ]); + // Compute the effective path for data link operations. + // For symlinks, construct the unresolved path from parent + symlink name + // (propertiesTarget.path is the resolved target path, not the symlink location). + // For non-symlinks, use propertiesTarget.path directly. + const dataLinkPath = useMemo(() => { + const parentPath = fileQuery.data?.currentFileOrFolder?.path; + if (!propertiesTarget) { + return parentPath || null; + } + if (propertiesTarget.is_symlink && propertiesTarget.name && parentPath) { + return parentPath === '.' + ? propertiesTarget.name + : `${parentPath}/${propertiesTarget.name}`; + } + return propertiesTarget.path; + }, [propertiesTarget, fileQuery.data?.currentFileOrFolder?.path]); + return ( ; + currentDirProxiedPathQuery: ReturnType< + typeof useProxiedPathByFspAndPathQuery + >; createProxiedPathMutation: ReturnType; deleteProxiedPathMutation: ReturnType; }; @@ -46,21 +49,34 @@ export const ProxiedPathProvider = ({ }: { readonly children: ReactNode; }) => { - const { fileQuery } = useFileBrowserContext(); + const { fileQuery, fileBrowserState } = useFileBrowserContext(); + + const isReady = !fileQuery.isPending && !fileQuery.isError; // Initialize all queries and mutations const allProxiedPathsQuery = useAllProxiedPathsQuery(); + + // Query for the properties target (used by the data link toggle in PropertiesDrawer) const proxiedPathByFspAndPathQuery = useProxiedPathByFspAndPathQuery( + fileQuery.data?.currentFileSharePath?.name, + fileBrowserState.dataLinkPath ?? undefined, + isReady + ); + + // Query for the current browsed directory (used by viewer icon URLs) + const currentDirProxiedPathQuery = useProxiedPathByFspAndPathQuery( fileQuery.data?.currentFileSharePath?.name, fileQuery.data?.currentFileOrFolder?.path, - !fileQuery.isPending && !fileQuery.isError + isReady ); + const createProxiedPathMutation = useCreateProxiedPathMutation(); const deleteProxiedPathMutation = useDeleteProxiedPathMutation(); const value: ProxiedPathContextType = { allProxiedPathsQuery, proxiedPathByFspAndPathQuery, + currentDirProxiedPathQuery, createProxiedPathMutation, deleteProxiedPathMutation }; diff --git a/frontend/src/hooks/useDataToolLinks.ts b/frontend/src/hooks/useDataToolLinks.ts index fe5991f1..f882c69b 100644 --- a/frontend/src/hooks/useDataToolLinks.ts +++ b/frontend/src/hooks/useDataToolLinks.ts @@ -19,7 +19,7 @@ export default function useDataToolLinks( pendingToolKey: PendingToolKey, setPendingToolKey: Dispatch> ): { - handleCreateDataLink: () => Promise; + handleCreateDataLink: (pathOverride?: string) => Promise; handleDeleteDataLink: (proxiedPath: ProxiedPath) => Promise; handleToolClick: (toolKey: PendingToolKey) => Promise; handleDialogConfirm: () => Promise; @@ -31,7 +31,7 @@ export default function useDataToolLinks( export default function useDataToolLinks( setShowDataLinkDialog: Dispatch> ): { - handleCreateDataLink: () => Promise; + handleCreateDataLink: (pathOverride?: string) => Promise; handleDeleteDataLink: (proxiedPath: ProxiedPath) => Promise; handleToolClick: (toolKey: PendingToolKey) => Promise; handleDialogConfirm: () => Promise; @@ -49,24 +49,27 @@ export default function useDataToolLinks( const currentUrlsRef = useRef(openWithToolUrls); currentUrlsRef.current = openWithToolUrls; - const { fileQuery } = useFileBrowserContext(); + const { fileQuery, fileBrowserState } = useFileBrowserContext(); const { createProxiedPathMutation, deleteProxiedPathMutation, allProxiedPathsQuery, - proxiedPathByFspAndPathQuery + currentDirProxiedPathQuery } = useProxiedPathContext(); const { areDataLinksAutomatic } = usePreferencesContext(); const { externalDataUrlQuery } = useExternalBucketContext(); const { handleCopy, showCopiedTooltip } = useCopyTooltip(); - const handleCreateDataLink = async (): Promise => { + const handleCreateDataLink = async ( + pathOverride?: string + ): Promise => { + const path = pathOverride || fileBrowserState.dataLinkPath; if (!fileQuery.data?.currentFileSharePath) { toast.error('No file share path selected'); return false; } - if (!fileQuery.data?.currentFileOrFolder) { + if (!path) { toast.error('No folder selected'); return false; } @@ -74,7 +77,7 @@ export default function useDataToolLinks( try { await createProxiedPathMutation.mutateAsync({ fsp_name: fileQuery.data.currentFileSharePath.name, - path: fileQuery.data.currentFileOrFolder.path + path }); toast.success('Data link created successfully'); await allProxiedPathsQuery.refetch(); @@ -134,7 +137,11 @@ export default function useDataToolLinks( return; } - const success = await handleCreateDataLink(); + // Viewer icon flow: always create data link for the current directory, + // not the properties target (which may be a selected subdirectory row) + const success = await handleCreateDataLink( + fileQuery.data?.currentFileOrFolder?.path + ); if (!success) { // If link creation fails, exit immediately without waiting or showing navigation error return; @@ -164,7 +171,7 @@ export default function useDataToolLinks( }; const handleToolClick = async (toolKey: PendingToolKey) => { - if (!proxiedPathByFspAndPathQuery.data && !externalDataUrlQuery.data) { + if (!currentDirProxiedPathQuery.data && !externalDataUrlQuery.data) { if (areDataLinksAutomatic) { await createLinkAndExecuteAction(toolKey); } else { @@ -172,7 +179,7 @@ export default function useDataToolLinks( setShowDataLinkDialog?.(true); } } else if ( - (proxiedPathByFspAndPathQuery.data || externalDataUrlQuery.data) && + (currentDirProxiedPathQuery.data || externalDataUrlQuery.data) && openWithToolUrls ) { await executeToolAction(toolKey, openWithToolUrls); diff --git a/frontend/src/hooks/useN5Metadata.ts b/frontend/src/hooks/useN5Metadata.ts index f432c1ce..b56bf955 100644 --- a/frontend/src/hooks/useN5Metadata.ts +++ b/frontend/src/hooks/useN5Metadata.ts @@ -49,7 +49,7 @@ function generateNeuroglancerStateForN5(dataUrl: string): string { export default function useN5Metadata() { const { fileQuery } = useFileBrowserContext(); - const { proxiedPathByFspAndPathQuery } = useProxiedPathContext(); + const { currentDirProxiedPathQuery } = useProxiedPathContext(); const { externalDataUrlQuery } = useExternalBucketContext(); // Fetch N5 metadata @@ -69,7 +69,7 @@ export default function useN5Metadata() { const neuroglancerBaseUrl = 'https://neuroglancer-demo.appspot.com/#!'; const url = - externalDataUrlQuery.data || proxiedPathByFspAndPathQuery.data?.url; + externalDataUrlQuery.data || currentDirProxiedPathQuery.data?.url; const toolUrls: N5OpenWithToolUrls = { copy: url || '', @@ -88,7 +88,7 @@ export default function useN5Metadata() { return toolUrls; }, [ metadata, - proxiedPathByFspAndPathQuery.data?.url, + currentDirProxiedPathQuery.data?.url, externalDataUrlQuery.data ]); diff --git a/frontend/src/hooks/useZarrMetadata.ts b/frontend/src/hooks/useZarrMetadata.ts index 3df57288..c0f59065 100644 --- a/frontend/src/hooks/useZarrMetadata.ts +++ b/frontend/src/hooks/useZarrMetadata.ts @@ -24,7 +24,7 @@ export type ZarrArray = zarr.Array; export default function useZarrMetadata() { const { fileQuery } = useFileBrowserContext(); - const { proxiedPathByFspAndPathQuery } = useProxiedPathContext(); + const { currentDirProxiedPathQuery } = useProxiedPathContext(); const { externalDataUrlQuery } = useExternalBucketContext(); const { disableNeuroglancerStateGeneration, @@ -97,7 +97,7 @@ export default function useZarrMetadata() { const avivatorBaseUrl = 'https://janeliascicomp.github.io/viv/'; const url = - externalDataUrlQuery.data || proxiedPathByFspAndPathQuery.data?.url; + externalDataUrlQuery.data || currentDirProxiedPathQuery.data?.url; const openWithToolUrls = { copy: url || '' } as OpenWithToolUrls; @@ -190,7 +190,7 @@ export default function useZarrMetadata() { return openWithToolUrls; }, [ metadata, - proxiedPathByFspAndPathQuery.data?.url, + currentDirProxiedPathQuery.data?.url, externalDataUrlQuery.data, disableNeuroglancerStateGeneration, useLegacyMultichannelApproach, From 69665a12beb3d00c061aca0e41ca8312661f9066 Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 11 Feb 2026 15:36:33 -0500 Subject: [PATCH 38/39] tests: data link targets selected subdirectory; viewer icon targets current directory --- .../tests/data-link-operations.spec.ts | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/frontend/ui-tests/tests/data-link-operations.spec.ts b/frontend/ui-tests/tests/data-link-operations.spec.ts index e81bd560..6b428d3c 100644 --- a/frontend/ui-tests/tests/data-link-operations.spec.ts +++ b/frontend/ui-tests/tests/data-link-operations.spec.ts @@ -152,4 +152,110 @@ test.describe('Data Link Operations', () => { ).toBeVisible(); }); }); + + test('Data link created for subdirectory clicked from parent shows subdirectory name', async ({ + fileglancerPage: page + }) => { + const zarrDirName = ZARR_TEST_FILE_INFO.v3_ome.dirname; + + const propertiesPanel = page + .locator('[role="complementary"]') + .filter({ hasText: 'Properties' }); + + // Click the row's Type column to select the subdirectory without navigating in + const zarrRow = page.getByRole('row').filter({ hasText: zarrDirName }); + await zarrRow.getByText('Folder', { exact: true }).click(); + + // Wait for properties panel to show the subdirectory name + await expect( + propertiesPanel.getByText(zarrDirName, { exact: true }) + ).toBeVisible({ timeout: 10000 }); + + // Wait for the data link toggle to appear + const dataLinkToggle = propertiesPanel.getByRole('checkbox', { + name: /data link/i + }); + await expect(dataLinkToggle).toBeVisible({ timeout: 10000 }); + + // Click the toggle to create a data link + await dataLinkToggle.click(); + + // Confirm in dialog + const confirmButton = page.getByRole('button', { + name: /confirm|create|yes/i + }); + await expect(confirmButton).toBeVisible({ timeout: 5000 }); + await confirmButton.click(); + + await expect( + page.getByText('Data link created successfully') + ).toBeVisible(); + + // Navigate to the Data links page + const linksNavButton = page.getByRole('link', { name: 'Data links' }); + await linksNavButton.click(); + + await expect(page.getByRole('heading', { name: /links/i })).toBeVisible(); + + // Verify the data link has the subdirectory name, not the parent directory name + const linkRow = page.getByText(zarrDirName, { exact: true }); + await expect(linkRow).toBeVisible({ timeout: 10000 }); + }); + + test('Viewer icon creates data link for current directory even when subdirectory row is selected', async ({ + fileglancerPage: page, + testDir + }) => { + const zarrDirName = ZARR_TEST_FILE_INFO.v3_ome.dirname; + + // Navigate into the zarr directory + await page.getByRole('link', { name: zarrDirName }).click(); + await expect(page.getByText('zarr.json')).toBeVisible({ timeout: 10000 }); + await expect( + page.getByRole('link', { name: 'Neuroglancer logo' }) + ).toBeVisible({ timeout: 10000 }); + + // Click on the s0 subdirectory row to select it as the properties target + const s0Row = page.getByRole('row').filter({ hasText: 's0' }); + await s0Row.getByText('Folder', { exact: true }).click(); + + // Verify s0 is selected in properties panel + const propertiesPanel = page + .locator('[role="complementary"]') + .filter({ hasText: 'Properties' }); + await expect(propertiesPanel.getByText('s0', { exact: true })).toBeVisible({ + timeout: 10000 + }); + + // Click the Neuroglancer viewer icon — this should create a data link + // for the zarr directory (currentFileOrFolder), not for s0 (propertiesTarget) + const neuroglancerLink = page.getByRole('link', { + name: 'Neuroglancer logo' + }); + await neuroglancerLink.click(); + + // Confirm in dialog + const confirmButton = page.getByRole('button', { + name: /confirm|create|yes/i + }); + await expect(confirmButton).toBeVisible({ timeout: 5000 }); + await confirmButton.click(); + + await expect( + page.getByText('Data link created successfully') + ).toBeVisible(); + + // Navigate back to check the data link on the links page + await page.goto('/browse', { waitUntil: 'domcontentloaded' }); + const linksNavButton = page.getByRole('link', { name: 'Data links' }); + await linksNavButton.click(); + + await expect(page.getByRole('heading', { name: /links/i })).toBeVisible(); + + // The data link should be for the zarr directory, not the s0 subdirectory + await expect(page.getByText(zarrDirName, { exact: true })).toBeVisible({ + timeout: 10000 + }); + await expect(page.getByText('s0', { exact: true })).not.toBeVisible(); + }); }); From b8e2c54585e71b670865ab2142271a5fa44bb0bf Mon Sep 17 00:00:00 2001 From: allison-truhlar Date: Wed, 11 Feb 2026 15:36:49 -0500 Subject: [PATCH 39/39] feat: clear file browser row selection with Escape key --- .../src/components/ui/BrowsePage/FileBrowser.tsx | 15 ++++++++++++++- frontend/src/contexts/FileBrowserContext.tsx | 12 +++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/ui/BrowsePage/FileBrowser.tsx b/frontend/src/components/ui/BrowsePage/FileBrowser.tsx index ab0e24b8..5d720ff9 100644 --- a/frontend/src/components/ui/BrowsePage/FileBrowser.tsx +++ b/frontend/src/components/ui/BrowsePage/FileBrowser.tsx @@ -1,3 +1,4 @@ +import { useEffect } from 'react'; import type { MouseEvent } from 'react'; import { Typography } from '@material-tailwind/react'; import toast from 'react-hot-toast'; @@ -52,7 +53,8 @@ export default function FileBrowser({ fspName, fileQuery, fileBrowserState, - updateFilesWithContextMenuClick + updateFilesWithContextMenuClick, + clearSelection } = useFileBrowserContext(); const { folderPreferenceMap, handleContextMenuFavorite } = usePreferencesContext(); @@ -83,6 +85,17 @@ export default function FileBrowser({ const isN5Dir = detectN5(fileQuery.data?.files as FileOrFolder[]); + // Escape key clears row selection and reverts properties to current directory + useEffect(() => { + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') { + clearSelection(); + } + }; + document.addEventListener('keydown', handleKeyDown); + return () => document.removeEventListener('keydown', handleKeyDown); + }, [clearSelection]); + // Handle right-click on file - FileBrowser-specific logic const handleFileContextMenu = ( e: MouseEvent, diff --git a/frontend/src/contexts/FileBrowserContext.tsx b/frontend/src/contexts/FileBrowserContext.tsx index ad6b9de9..151f7c54 100644 --- a/frontend/src/contexts/FileBrowserContext.tsx +++ b/frontend/src/contexts/FileBrowserContext.tsx @@ -81,6 +81,7 @@ type FileBrowserContextType = { showFilePropertiesDrawer: boolean ) => void; updateFilesWithContextMenuClick: (file: FileOrFolder) => void; + clearSelection: () => void; }; const FileBrowserContext = createContext(null); @@ -182,6 +183,14 @@ export const FileBrowserContextProvider = ({ }); }; + const clearSelection = useCallback(() => { + setInternalState({ + propertiesTargetPath: fileQuery.data?.currentFileOrFolder?.path || null, + propertiesTargetName: null, + selectedFiles: [] + }); + }, [fileQuery.data?.currentFileOrFolder?.path]); + // Update client state when URL changes (navigation to different file/folder) // Set propertiesTarget to the current directory/file being viewed useEffect( @@ -319,7 +328,8 @@ export const FileBrowserContextProvider = ({ // Actions handleLeftClick, - updateFilesWithContextMenuClick + updateFilesWithContextMenuClick, + clearSelection }} > {children}