diff --git a/frontend/src/__tests__/components/ModelViewerModal.test.tsx b/frontend/src/__tests__/components/ModelViewerModal.test.tsx index e41eadab0..62c0e4bdb 100644 --- a/frontend/src/__tests__/components/ModelViewerModal.test.tsx +++ b/frontend/src/__tests__/components/ModelViewerModal.test.tsx @@ -29,12 +29,13 @@ vi.mock('../../components/GcodeViewer', () => ({ ), })); -vi.mock('../../utils/slicer', () => ({ +// Only the protocol-handler launch is stubbed — it would navigate the jsdom +// window. Everything else in the module is a pure predicate, so keep the real +// implementations: re-declaring them here would let the file-type rule these +// tests assert on drift away from the one the component actually runs. +vi.mock('../../utils/slicer', async (importOriginal) => ({ + ...(await importOriginal()), openInSlicer: vi.fn(), - resolveDesktopSlicer: vi.fn( - (openInSlicer?: string, preferredSlicer?: string) => - (openInSlicer ?? preferredSlicer ?? 'bambu_studio') as 'bambu_studio' | 'orcaslicer', - ), })); const mockCapabilities = { diff --git a/frontend/src/__tests__/pages/FileManagerPage.test.tsx b/frontend/src/__tests__/pages/FileManagerPage.test.tsx index 080230936..a07de5309 100644 --- a/frontend/src/__tests__/pages/FileManagerPage.test.tsx +++ b/frontend/src/__tests__/pages/FileManagerPage.test.tsx @@ -12,12 +12,13 @@ import { setAuthToken } from '../../api/client'; import { http, HttpResponse } from 'msw'; import { server } from '../mocks/server'; -vi.mock('../../utils/slicer', () => ({ +// Only the protocol-handler launch is stubbed — it would navigate the jsdom +// window. Everything else in the module is a pure predicate, so keep the real +// implementations: isSliceableFilename decides which rows even offer the +// action these tests click. +vi.mock('../../utils/slicer', async (importOriginal) => ({ + ...(await importOriginal()), openInSlicer: vi.fn(), - resolveDesktopSlicer: vi.fn( - (openInSlicer?: string, preferredSlicer?: string) => - (openInSlicer ?? preferredSlicer ?? 'bambu_studio') as 'bambu_studio' | 'orcaslicer', - ), })); vi.mock('../../components/SliceModal', () => ({ @@ -1239,11 +1240,12 @@ describe('FileManagerPage', () => { expect(within(card).queryByText('Slice')).not.toBeInTheDocument(); }); - // Permission gating is the security-relevant half of the slice action: - // the in-app API path needs library:upload (the permission the backend - // enforces on the slicer-token endpoint), the desktop handoff mirrors the - // backend's ownership check and needs library:read_all / library:read_own - // (legacy library:read also accepted). + // Permission gating is the security-relevant half of the slice action: the + // in-app API path needs library:upload, and the desktop handoff mirrors the + // ownership check the slicer-token endpoint runs — library:read_all or + // library:read_own. The legacy library:read is deliberately not accepted; + // it satisfies neither the token endpoint nor the folder listing that gets + // a user to this page at all. const mockAuthUser = (permissions: string[]) => { setAuthToken('test-token', 'session'); server.use( @@ -1322,6 +1324,26 @@ describe('FileManagerPage', () => { expect(sliceItem).not.toBeDisabled(); }); + it('does not accept the legacy library:read for the desktop handoff', async () => { + // require_ownership_permission(LIBRARY_READ_ALL, LIBRARY_READ_OWN) does no + // legacy expansion, so this group 403s on the slicer-token endpoint. + // Enabling the item would offer an action the server refuses, and the + // failure would look like "no slicer installed" once the fallback URL is + // handed over. + mockAuthUser(['library:read']); + const user = userEvent.setup(); + render(); + + await waitFor(() => expect(screen.getByText('bracket.stl')).toBeInTheDocument()); + + const card = await openMenu(user, 'bracket.stl'); + const sliceItem = within(card).getByText('Slice').closest('button'); + expect(sliceItem).toBeDisabled(); + + await user.click(sliceItem!); + expect(openInSlicer).not.toHaveBeenCalled(); + }); + it('slices from the list-view button when the slicer API is disabled', async () => { const user = userEvent.setup(); render(); diff --git a/frontend/src/components/ModelViewerModal.tsx b/frontend/src/components/ModelViewerModal.tsx index 2d91a81e0..9f12a0953 100644 --- a/frontend/src/components/ModelViewerModal.tsx +++ b/frontend/src/components/ModelViewerModal.tsx @@ -7,7 +7,7 @@ import { GcodeViewer } from './GcodeViewer'; import { Button } from './Button'; import { api, withStreamToken } from '../api/client'; import { useToast } from '../contexts/ToastContext'; -import { openInSlicer, resolveDesktopSlicer, type SlicerType } from '../utils/slicer'; +import { isSliceableFileType, openInSlicer, resolveDesktopSlicer, type SlicerType } from '../utils/slicer'; import type { ArchivePlatesResponse, LibraryFilePlatesResponse, PlateMetadata } from '../types/plates'; type ViewTab = '3d' | 'gcode'; @@ -373,11 +373,11 @@ export function ModelViewerModal({ archiveId, libraryFileId, title, fileType, on }, [isDraggingDivider, dividerHeight, minPlateHeight, minViewerPx, minViewerRatio]); // Which file types can be handed to a desktop slicer via the URL protocol - // handler — and sliced in-app via the sidecar. Kept in step with - // `isSliceableFilename()` in FileManagerPage so a file's card-menu "Slice" - // and its 3D-preview slicer button never disagree on the same type. - const normalizedFileType = (fileType || '').toLowerCase(); - const slicerReadyType = ['3mf', 'stl', 'step', 'stp'].includes(normalizedFileType); + // handler — and sliced in-app via the sidecar. Shares its list with + // `isSliceableFilename()`, which the File Manager's card menu and list row + // use, so a file's "Slice" action and its 3D-preview slicer button can no + // longer disagree about the same file. + const slicerReadyType = isSliceableFileType(fileType); const canOpenInSlicer = isLibrary ? slicerReadyType : true; // When the user has the in-app Slicer API enabled (Settings → Workflow → diff --git a/frontend/src/pages/FileManagerPage.tsx b/frontend/src/pages/FileManagerPage.tsx index c3425bac7..1fa82151a 100644 --- a/frontend/src/pages/FileManagerPage.tsx +++ b/frontend/src/pages/FileManagerPage.tsx @@ -74,7 +74,7 @@ import { usePageFileDrop } from '../hooks/usePageFileDrop'; import { useAuth } from '../contexts/AuthContext'; import { formatDuration, parseUTCDate, formatDate } from '../utils/date'; import { formatFileSize } from '../utils/file'; -import { openInSlicer, resolveDesktopSlicer, type SlicerType } from '../utils/slicer'; +import { isSliceableFilename, openInSlicer, resolveDesktopSlicer, type SlicerType } from '../utils/slicer'; type SortField = 'name' | 'date' | 'size' | 'type' | 'prints'; type SortDirection = 'asc' | 'desc'; @@ -743,14 +743,6 @@ function isSlicedFilename(filename: string): boolean { return lower.endsWith('.gcode') || lower.endsWith('.gcode.3mf'); } -// Files that can be fed to the slicer sidecar (model geometry inputs). -// Excludes .gcode.* (already sliced) and any other non-model formats. -function isSliceableFilename(filename: string): boolean { - const lower = filename.toLowerCase(); - if (lower.endsWith('.gcode') || lower.endsWith('.gcode.3mf')) return false; - return lower.endsWith('.stl') || lower.endsWith('.3mf') || lower.endsWith('.step') || lower.endsWith('.stp'); -} - // File Card interface FileCardProps { file: LibraryFileListItem; @@ -1172,16 +1164,26 @@ export function FileManagerPage() { } }, [preferredSlicer, showToast, t]); - // Slice permission: API mode needs upload rights, desktop handoff is a download. - // The handoff mirrors the backend's ownership check on the slicer-token - // endpoint (library:read_all / library:read_own); `library:read` is a legacy - // permission default groups don't carry, so requiring it would disable the - // handoff for Operators and Viewers. + // Slice permission: API mode needs upload rights, the desktop handoff is a + // download. Each mirrors what the backend enforces on the endpoint that + // branch actually calls, so the UI never offers an action the server refuses. + // + // Deliberately NOT accepting the legacy `library:read` on the handoff branch. + // It looks like the safe back-compat term to include, but the slicer-token + // endpoint gates on require_ownership_permission(LIBRARY_READ_ALL, + // LIBRARY_READ_OWN), and neither that dependency nor User.has_permission + // expands the legacy name — so a group holding only `library:read` gets a 403 + // there. It cannot reach this page to find out either: GET /library/folders + // gates on the same pair. Accepting it here would only enable a menu item + // that fails, and the `library:read` -> `library:read_own` migration in + // core/database.py runs only over the groups named in DEFAULT_GROUPS, so a + // custom role that still carries it is genuinely stuck rather than silently + // upgraded. const canSlice = useCallback(() => { if (settings?.use_slicer_api) { return hasPermission('library:upload'); } - return hasAnyPermission('library:read_all', 'library:read_own', 'library:read'); + return hasAnyPermission('library:read_all', 'library:read_own'); }, [settings?.use_slicer_api, hasPermission, hasAnyPermission]); const { data: folders, isLoading: foldersLoading } = useQuery({ queryKey: ['library-folders'], diff --git a/frontend/src/utils/slicer.ts b/frontend/src/utils/slicer.ts index ad6a71f5a..b1fd719e3 100644 --- a/frontend/src/utils/slicer.ts +++ b/frontend/src/utils/slicer.ts @@ -41,6 +41,43 @@ export function resolveDesktopSlicer( return openInSlicer ?? preferredSlicer ?? 'bambu_studio'; } +/** + * File types a slicer can be handed — both by the desktop URI handler and by + * the in-app sidecar. Source geometry only: a sliced file is an output, and + * neither slicer has anything to do with one. + * + * Lives here rather than beside either caller because both the File Manager + * (which has a filename) and the 3D preview (which has a `LibraryFile.file_type`) + * decide the same thing about the same file. They used to hold separate lists, + * and the two disagreed — a card menu offered a desktop handoff for an STL + * whose own 3D preview showed "Open in Slicer" greyed out. + */ +export const SLICEABLE_FILE_TYPES = ['3mf', 'stl', 'step', 'stp'] as const; + +/** + * Does a `LibraryFile.file_type` name a sliceable source file? + * + * The backend stores compound extensions whole — a sliced 3MF classifies as + * `gcode.3mf`, not `3mf` (`classify_file_type` in `api/routes/library.py`) — so + * membership alone is enough to exclude sliced output here. + */ +export function isSliceableFileType(fileType?: string | null): boolean { + const normalized = (fileType || '').toLowerCase(); + return (SLICEABLE_FILE_TYPES as readonly string[]).includes(normalized); +} + +/** + * Does a filename name a sliceable source file? + * + * Checked against the name rather than a stored type, so the compound + * extensions have to be ruled out explicitly: `.gcode.3mf` ends with `.3mf`. + */ +export function isSliceableFilename(filename: string): boolean { + const lower = filename.toLowerCase(); + if (lower.endsWith('.gcode') || lower.endsWith('.gcode.3mf')) return false; + return SLICEABLE_FILE_TYPES.some((ext) => lower.endsWith(`.${ext}`)); +} + /** * Detect the user's operating system */