From 3c76aadc04e20e435aff1c682ac6f04e5d6e0749 Mon Sep 17 00:00:00 2001 From: F3l1x1vo Date: Mon, 10 Aug 2026 13:54:22 +0200 Subject: [PATCH 01/48] #224 add tooltips to many locations --- messages/de.json | 3 + messages/en.json | 3 + src/lib/components/Pagination.svelte | 140 ++++++++------- src/lib/components/TabBar.svelte | 79 +++++---- src/lib/components/ToastHost.svelte | 17 +- src/lib/components/Tooltip.svelte | 21 ++- src/lib/components/TooltipTrigger.svelte | 118 +++++++++++++ .../components/TooltipTrigger.svelte.spec.ts | 74 ++++++++ .../TooltipTriggerDialogWrapper.svelte | 18 ++ .../__tests__/TooltipTriggerWrapper.svelte | 13 ++ .../components/catalog/CatalogBrowser.svelte | 41 ++--- .../layout/header/LanguageSwitcher.svelte | 35 ++-- .../layout/header/ThemeToggle.svelte | 25 +-- .../components/layout/sidebar/Sidebar.svelte | 83 ++++----- .../storage/explorer/ContextMenu.svelte | 20 ++- .../storage/explorer/FileRow.svelte | 18 +- .../storage/explorer/FolderRow.svelte | 18 +- .../storage/explorer/OperationsButton.svelte | 70 ++++---- .../storage/explorer/StorageBreadcrumb.svelte | 83 +++++---- .../components/storage/explorer/TabBar.svelte | 53 +++--- .../storage/landing/BucketGrid.svelte | 41 ++--- .../modals/DeleteConnectionModal.svelte | 32 ++-- .../storage/modals/DetailsModal.svelte | 42 +++-- .../storage/modals/PreviewModal.svelte | 44 +++-- .../storage/modals/upload/UploadModal.svelte | 16 +- .../storage/sidebar/BucketList.svelte | 165 ++++++++---------- .../sidebar/StorageConnectionSidebar.svelte | 103 +++++------ src/routes/(app)/storage/+page.svelte | 16 +- .../(app)/storage/connections/+page.svelte | 19 +- src/routes/(app)/trino/+page.svelte | 3 +- 30 files changed, 837 insertions(+), 576 deletions(-) create mode 100644 src/lib/components/TooltipTrigger.svelte create mode 100644 src/lib/components/TooltipTrigger.svelte.spec.ts create mode 100644 src/lib/components/__tests__/TooltipTriggerDialogWrapper.svelte create mode 100644 src/lib/components/__tests__/TooltipTriggerWrapper.svelte diff --git a/messages/de.json b/messages/de.json index f9e38d66..9dd9ff9e 100644 --- a/messages/de.json +++ b/messages/de.json @@ -1,5 +1,7 @@ { "$schema": "https://inlang.com/schema/inlang-message-format", + "action_close": "Schließen", + "action_dismiss": "Schließen", "dashboard_welcome": "Willkommen zurück", "dashboard_subtitle": "Stackable Unified Data Platform – Übersicht", "dashboard_services": "Dienste", @@ -251,6 +253,7 @@ "storage_paste_plural": "{count} Elemente einfügen", "storage_selected": "{count} ausgewählt", "storage_action_preview": "Vorschau", + "storage_action_actions_for": "Aktionen für {name}", "storage_action_refresh": "Aktualisieren", "storage_action_preview_no_selection": "Keine Datei für die Vorschau ausgewählt.", "storage_action_download_no_selection": "Keine Datei für den Download ausgewählt.", diff --git a/messages/en.json b/messages/en.json index 3be2ce03..97847e02 100644 --- a/messages/en.json +++ b/messages/en.json @@ -1,5 +1,7 @@ { "$schema": "https://inlang.com/schema/inlang-message-format", + "action_close": "Close", + "action_dismiss": "Dismiss", "dashboard_welcome": "Welcome back", "dashboard_subtitle": "Stackable Unified Data Platform overview", "dashboard_services": "Services", @@ -251,6 +253,7 @@ "storage_paste_plural": "Paste {count} items", "storage_selected": "{count} selected", "storage_action_preview": "Preview", + "storage_action_actions_for": "Actions for {name}", "storage_action_refresh": "Refresh", "storage_action_preview_no_selection": "No file selected for preview.", "storage_action_download_no_selection": "No file selected for download.", diff --git a/src/lib/components/Pagination.svelte b/src/lib/components/Pagination.svelte index 6aad8534..280d36c1 100644 --- a/src/lib/components/Pagination.svelte +++ b/src/lib/components/Pagination.svelte @@ -106,43 +106,47 @@
{#if showNavButtons} {/if} @@ -162,43 +166,47 @@ {infoLabel} {/if}
diff --git a/src/lib/components/TabBar.svelte b/src/lib/components/TabBar.svelte index 53586b2c..01187a23 100644 --- a/src/lib/components/TabBar.svelte +++ b/src/lib/components/TabBar.svelte @@ -1,5 +1,6 @@ {#if text} - - +
+ +
diff --git a/src/lib/components/storage/explorer/StorageBreadcrumb.svelte b/src/lib/components/storage/explorer/StorageBreadcrumb.svelte index 883812f4..2258e345 100644 --- a/src/lib/components/storage/explorer/StorageBreadcrumb.svelte +++ b/src/lib/components/storage/explorer/StorageBreadcrumb.svelte @@ -25,6 +25,7 @@ import { parseStorageDropKeys, canStorageDrop } from '$lib/storage/drag-handlers.js'; import OperationsButton from './OperationsButton.svelte'; import ContextMenu from './ContextMenu.svelte'; + import TooltipTrigger from '$lib/components/TooltipTrigger.svelte'; import type { ContextMenuAction } from '$lib/storage/types.js'; const storage = getStorageState(); @@ -265,34 +266,38 @@ motion-safe:transition-[width] motion-safe:duration-150 motion-safe:group-hover:delay-700 " > - + + {/snippet} @@ -376,18 +381,20 @@ collapsedDropdownOpen = false; }} > - +
+ +
+ {/if} + diff --git a/src/lib/components/storage/explorer/FileExplorer.svelte b/src/lib/components/storage/explorer/FileExplorer.svelte index 67e9655a..05a1b2c5 100644 --- a/src/lib/components/storage/explorer/FileExplorer.svelte +++ b/src/lib/components/storage/explorer/FileExplorer.svelte @@ -111,7 +111,9 @@ (storage.selectedFiles.length === 1 && storage.selectedFolders.length === 0) || (storage.contextMenu !== null && storage.ctxIsFile) ); - const canDownload = $derived(storage.selectedFiles.length > 0 || storage.ctxIsFile); + const canDownload = $derived( + storage.selectedFiles.length > 0 || storage.selectedFolders.length > 0 || storage.ctxIsFile + ); const canShowDetails = $derived( storage.contextMenu !== null || (storage.selectedFiles.length === 1 && storage.selectedFolders.length === 0) diff --git a/src/lib/components/storage/explorer/OperationsButton.svelte b/src/lib/components/storage/explorer/OperationsButton.svelte index 4d30d248..e35ad934 100644 --- a/src/lib/components/storage/explorer/OperationsButton.svelte +++ b/src/lib/components/storage/explorer/OperationsButton.svelte @@ -14,6 +14,8 @@ import IconDeleteSweep from 'virtual:icons/material-symbols/delete-sweep-outline'; import IconHistory from 'virtual:icons/material-symbols/history'; import IconChevronRight from 'virtual:icons/material-symbols/chevron-right'; + import IconDownload from 'virtual:icons/material-symbols/download'; + import DownloadHistory from './DownloadHistory.svelte'; const storage = getStorageState(); @@ -21,48 +23,12 @@ paste: IconContentPaste, move: IconDriveFileMove, rename: IconEdit, - delete: IconDelete + delete: IconDelete, + download: IconDownload }; let dropdownOpen = $state(false); - let tick = $state(0); let expandedOps: Record = $state({}); - let dropdownEl: HTMLDivElement | undefined = $state(); - - interface ChunkTrack { - prevBytes: number; - chunkStart: number; - chunkBytes: number; - } - let chunkTracks: Record = {}; - - $effect(() => { - if (!storage.hasRunningOps) return; - const hasByteProgress = storage.operations.some( - (op) => op.status === 'running' && op.totalBytes > 0 - ); - const intervalMs = hasByteProgress ? 100 : 1000; - const id = setInterval(() => { - tick++; - for (const op of storage.operations) { - if (op.status !== 'running' || op.totalBytes <= 0) continue; - const track = chunkTracks[op.id]; - if (!track) { - chunkTracks[op.id] = { - prevBytes: op.completedBytes, - chunkStart: Date.now(), - chunkBytes: 0 - }; - } else if (op.completedBytes !== track.prevBytes) { - const diff = op.completedBytes - track.prevBytes; - track.prevBytes = op.completedBytes; - track.chunkBytes = diff; - track.chunkStart = Date.now(); - } - } - }, intervalMs); - return () => clearInterval(id); - }); function toggleDropdown() { dropdownOpen = !dropdownOpen; @@ -80,6 +46,7 @@ .reverse() ); const hasHistory = $derived(historyOps.length > 0); + const hasAnyHistory = $derived(hasHistory || storage.downloadHistory.length > 0); const hasError = $derived(storage.operations.some((o) => o.status === 'error')); function statusColor(op: StorageOperation): string { @@ -121,12 +88,11 @@ case 'interrupted': return m.storage_operation_interrupted(); default: - return op.errorMessage ?? m.storage_operation_failed(); + return m.storage_operation_failed(); } } function formatElapsed(startedAt: number, completedAt: number | undefined): string { - void tick; const end = completedAt ?? Date.now(); const seconds = Math.floor((end - startedAt) / 1000); if (seconds < 60) return `${seconds}s`; @@ -144,22 +110,10 @@ } /** - * Return an interpolated completedBytes that smooths over chunk boundaries. - * When a new chunk completes (a jump in completedBytes), this converges - * smoothly from the previous value to the new one over ~1.5s instead of - * snapping, giving a smoother visual for multipart S3 copy operations - * where each part is 256 MB. + * Use the latest server-reported byte total for the progress display. */ function displayBytes(op: StorageOperation): number { - if (op.status !== 'running' || op.totalBytes <= 0) return op.completedBytes; - void tick; - const track = chunkTracks[op.id]; - if (!track || track.chunkBytes <= 0) return op.completedBytes; - const elapsed = (Date.now() - track.chunkStart) / 1000; - const expectedDuration = 1.5; - const progress = Math.min(1, elapsed / expectedDuration); - const chunkProgress = track.chunkBytes * progress; - return Math.min(track.prevBytes, track.prevBytes - track.chunkBytes + chunkProgress); + return op.completedBytes; } function percent(op: StorageOperation): number { @@ -179,7 +133,6 @@ } function speedLabel(op: StorageOperation): string { - void tick; const elapsed = (Date.now() - op.startedAt) / 1000; if (elapsed <= 0 || op.completedBytes <= 0) return ''; const bytesPerSec = op.completedBytes / elapsed; @@ -187,7 +140,6 @@ } function etaLabel(op: StorageOperation): string { - void tick; const elapsed = (Date.now() - op.startedAt) / 1000; if (elapsed <= 0 || op.completedBytes <= 0 || op.totalBytes <= 0) return ''; const bytesPerSec = op.completedBytes / elapsed; @@ -211,7 +163,7 @@ e.key === 'Escape' && (dropdownOpen = false)} /> -{#if storage.operations.length > 0} +{#if storage.operations.length > 0 || storage.downloadHistory.length > 0 || storage.downloadHistoryLoading} {#if dropdownOpen} {/if} -
+
-
+
-
    - {#each historyOps as op (op.id)} - {@const TypeIcon = typeIconMap[op.type]} -
  • -
    - - - - - - {op.label} - -
    - - {statusLabel(op)} + {#if hasHistory} +
      + {#each historyOps as op (op.id)} + {@const TypeIcon = typeIconMap[op.type]} +
    • +
      + + - - {formatElapsed(op.startedAt, op.completedAt)} + + + + {op.label} +
      + + {statusLabel(op)} + + + {formatElapsed(op.startedAt, op.completedAt)} + +
      -
    - - {#if op.status === 'error' && op.errorMessage} -
    - {op.errorMessage} -
    - {:else if op.status === 'interrupted'} -
    - {m.storage_operations_interrupted_tooltip()} -
    - {/if} + + {#if op.status === 'error' && op.errorMessage} +
    + {op.errorMessage} +
    + {:else if op.status === 'interrupted'} +
    + {m.storage_operations_interrupted_tooltip()} +
    + {/if} - - {#if (op.status === 'interrupted' || op.status === 'error') && op.totalBytes > 0} -
    -
    -
    + + {#if (op.status === 'interrupted' || op.status === 'error') && op.totalBytes > 0} +
    +
    +
    +
    + + {formatBytes(op.completedBytes)} / {formatBytes(op.totalBytes)} +
    - - {formatBytes(op.completedBytes)} / {formatBytes(op.totalBytes)} - -
    - {/if} -
    • - {/each} -
+ {/if} + + {/each} + + {/if}
{:else if activeOps.length === 0}
@@ -498,6 +452,10 @@ {m.storage_operations_history()}
{/if} + storage.redownloadHistory(id, keys)} + />
{/if}
diff --git a/src/lib/components/storage/explorer/SelectionToolbar.svelte b/src/lib/components/storage/explorer/SelectionToolbar.svelte index 95fadff0..259dccc1 100644 --- a/src/lib/components/storage/explorer/SelectionToolbar.svelte +++ b/src/lib/components/storage/explorer/SelectionToolbar.svelte @@ -5,8 +5,9 @@ import IconDelete from 'virtual:icons/material-symbols/delete'; import IconRefresh from 'virtual:icons/material-symbols/refresh'; import * as m from '$lib/paraglide/messages.js'; - import { getStorageState } from '$lib/storage/context.js'; import StorageSearch from '$lib/components/storage/StorageSearch.svelte'; + import { getStorageState } from '$lib/storage/context.js'; + import { formatFileSize } from '$lib/storage/utils.js'; const storage = getStorageState(); @@ -14,8 +15,9 @@ const canPreview = $derived( storage.selectedFiles.length === 1 && storage.selectedFolders.length === 0 ); - const canDownload = $derived( - storage.selectedFiles.length === 1 && storage.selectedFolders.length === 0 + const canDownload = $derived(selectedCount > 0); + const selectedFileSize = $derived( + storage.selectedFiles.reduce((total, file) => total + file.size, 0) ); @@ -25,6 +27,11 @@ {m.storage_selected({ count: selectedCount })} + {#if storage.selectedFiles.length > 0} + + {m.storage_selected_files_size({ size: formatFileSize(selectedFileSize) })} + + {/if} {/if} - {/if} From dd5a02e5c8689f933e3bbda3b5c637704753c8d1 Mon Sep 17 00:00:00 2001 From: F3l1x1vo Date: Mon, 31 Aug 2026 11:08:39 +0200 Subject: [PATCH 13/48] fix lint --- e2e/storage/archive.spec.ts | 2 +- src/lib/components/storage/explorer/StorageBreadcrumb.svelte | 2 -- src/lib/storage/connection-store.svelte.ts | 4 +++- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/e2e/storage/archive.spec.ts b/e2e/storage/archive.spec.ts index 9bc34feb..ae240cf0 100644 --- a/e2e/storage/archive.spec.ts +++ b/e2e/storage/archive.spec.ts @@ -169,7 +169,7 @@ test.describe('Storage S3 — Archive preview', () => { await waitForObjectsLoaded(page); // Should be at the bucket root (archive exited) - // eslint-disable-next-line security/detect-non-literal-regexp + await expect(page).toHaveURL( new RegExp( `/storage/browse/${new URL(credentials.endpoint).hostname}/${credentials.bucket}(\\?|$)` diff --git a/src/lib/components/storage/explorer/StorageBreadcrumb.svelte b/src/lib/components/storage/explorer/StorageBreadcrumb.svelte index be9d683b..18b9ac86 100644 --- a/src/lib/components/storage/explorer/StorageBreadcrumb.svelte +++ b/src/lib/components/storage/explorer/StorageBreadcrumb.svelte @@ -15,7 +15,6 @@ import IconContentCopy from 'virtual:icons/material-symbols/content-copy'; import * as m from '$lib/paraglide/messages.js'; import { getStorageState } from '$lib/storage/context.js'; - import { getTabsState } from '$lib/storage/context.js'; import type { StorageLocation } from '$lib/storage/types.js'; import { keyToName } from '$lib/storage/utils.js'; import { invalidateAll } from '$app/navigation'; @@ -27,7 +26,6 @@ import type { ContextMenuAction } from '$lib/storage/types.js'; const storage = getStorageState(); - const tabsState = getTabsState(); const breadcrumbParts = $derived( storage.prefix diff --git a/src/lib/storage/connection-store.svelte.ts b/src/lib/storage/connection-store.svelte.ts index 89fbee80..8c9da142 100644 --- a/src/lib/storage/connection-store.svelte.ts +++ b/src/lib/storage/connection-store.svelte.ts @@ -5,6 +5,8 @@ * This module is client-safe. It never imports server-only code. */ +import { SvelteURL } from 'svelte/reactivity'; + export interface ConnectionListItem { id: string; name: string; @@ -17,7 +19,7 @@ export interface ConnectionListItem { export function connectionHostname(connection: ConnectionListItem | null): string { if (!connection?.endpoint) return ''; try { - return new URL( + return new SvelteURL( connection.endpoint.includes('://') ? connection.endpoint : `//${connection.endpoint}`, 'http://localhost' ).hostname; From 3ed1e0e58fdad43abc1d1172d19656eac520db42 Mon Sep 17 00:00:00 2001 From: F3l1x1vo Date: Mon, 31 Aug 2026 14:50:42 +0200 Subject: [PATCH 14/48] fix history.pushState --- src/routes/(app)/storage/+layout.svelte | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/(app)/storage/+layout.svelte b/src/routes/(app)/storage/+layout.svelte index 4bc54c7b..1c5fb50f 100644 --- a/src/routes/(app)/storage/+layout.svelte +++ b/src/routes/(app)/storage/+layout.svelte @@ -1,6 +1,6 @@ -
+

{m.storage_download_history_title()}

@@ -53,32 +56,34 @@ {@const totalSize = entry.entries.reduce((total, item) => total + item.size, 0)} {@const selectedSize = selectedDownloadHistoryPayloadSize(entry.entries, chosen)}
  • -
    -
    -

    {entry.bucket}

    -

    +

    - -
    + + {#if expanded[entry.id]}
    From 81f58023f7721c9ff25bb50ac4102d6de0f37831 Mon Sep 17 00:00:00 2001 From: dklOrdix Date: Wed, 2 Sep 2026 15:12:56 +0200 Subject: [PATCH 20/48] fix: report partial storage search failures --- messages/de.json | 5 +- messages/en.json | 5 +- .../components/storage/StorageSearch.svelte | 2 + .../storage/StorageSearchResults.svelte | 19 +++-- .../storage/StorageSearchSessions.svelte | 7 +- .../__tests__/StorageSearch.svelte.spec.ts | 24 ++++++ .../server/storage/s3-provider-search.test.ts | 14 ++++ src/lib/server/storage/s3-provider.ts | 81 ++++++++++--------- src/lib/storage/search-stream.spec.ts | 7 ++ src/lib/storage/search-stream.ts | 6 +- src/lib/storage/search.svelte.ts | 42 ++++++++-- .../(app)/api/storage/search/+server.ts | 12 ++- .../(app)/api/storage/search/server.test.ts | 15 ++++ 13 files changed, 184 insertions(+), 55 deletions(-) diff --git a/messages/de.json b/messages/de.json index 279c2790..0999eaef 100644 --- a/messages/de.json +++ b/messages/de.json @@ -700,15 +700,18 @@ "storage_search_tab_recent": "Letzte", "storage_search_no_results": "Keine Ergebnisse gefunden.", "storage_search_truncated": "Es werden nur die ersten passenden Ergebnisse angezeigt.", - "storage_search_error": "Die Suche konnte nicht abgeschlossen werden. Bitte versuchen Sie es erneut.", + "storage_search_failure_access_denied": "Der Zugriff auf einen oder mehrere ausgewählte Buckets wurde verweigert.", + "storage_search_failure_generic": "Ein oder mehrere ausgewählte Buckets konnten nicht durchsucht werden. Bitte versuchen Sie es erneut.", "storage_search_session_label": "Suche {number}", "storage_search_running": "{count} aktiv", "storage_search_completed": "{count} abgeschlossen", + "storage_search_partial_count": "{count} teilweise", "storage_search_add_session": "Parallele Suche", "storage_search_sessions_label": "Suchsitzungen", "storage_search_remove_session": "{name} entfernen", "storage_search_status_running": "Aktiv", "storage_search_status_done": "Fertig", + "storage_search_status_partial": "Teilweise", "storage_search_status_error": "Fehler", "storage_search_regex_placeholder": "Regulärer Ausdruck", "storage_search_regex_badge": ".*", diff --git a/messages/en.json b/messages/en.json index e2148f0b..13699a67 100644 --- a/messages/en.json +++ b/messages/en.json @@ -700,15 +700,18 @@ "storage_search_tab_recent": "Recent", "storage_search_no_results": "No results found.", "storage_search_truncated": "Only the first matching results are shown.", - "storage_search_error": "Search could not be completed. Please try again.", + "storage_search_failure_access_denied": "Access to one or more selected buckets was denied.", + "storage_search_failure_generic": "One or more selected buckets could not be searched. Please try again.", "storage_search_session_label": "Search {number}", "storage_search_running": "{count} running", "storage_search_completed": "{count} completed", + "storage_search_partial_count": "{count} partial", "storage_search_add_session": "Parallel search", "storage_search_sessions_label": "Search sessions", "storage_search_remove_session": "Remove {name}", "storage_search_status_running": "Running", "storage_search_status_done": "Done", + "storage_search_status_partial": "Partial", "storage_search_status_error": "Error", "storage_search_regex_placeholder": "Regular expression", "storage_search_regex_badge": ".*", diff --git a/src/lib/components/storage/StorageSearch.svelte b/src/lib/components/storage/StorageSearch.svelte index 594c7584..d4daab58 100644 --- a/src/lib/components/storage/StorageSearch.svelte +++ b/src/lib/components/storage/StorageSearch.svelte @@ -103,6 +103,8 @@ {#if search.runningCount > 0}{m.storage_search_running({ count: search.runningCount })}{:else if search.partialCount > 0}{m.storage_search_partial_count({ count: search.partialCount })}{:else if search.completedCount > 0}{m.storage_search_completed({ count: search.completedCount })}{/if} diff --git a/src/lib/components/storage/StorageSearchResults.svelte b/src/lib/components/storage/StorageSearchResults.svelte index a3ab7975..bbdf2abc 100644 --- a/src/lib/components/storage/StorageSearchResults.svelte +++ b/src/lib/components/storage/StorageSearchResults.svelte @@ -52,6 +52,13 @@ }); }); + const failureMessage = $derived.by(() => { + if (session.failures.length === 0) return undefined; + return session.failures.every((failure) => failure.code === 'access_denied') + ? m.storage_search_failure_access_denied() + : m.storage_search_failure_generic(); + }); + function formatSize(size: number): string { if (size < 1024) return `${size} B`; if (size < 1024 ** 2) return `${(size / 1024).toFixed(1)} KB`; @@ -85,16 +92,18 @@ > {#if session.status === 'running'}{m.storage_search_status_running()}{:else if session.status === 'done'}{m.storage_search_status_done()}{:else if session.status === 'done' && session.failures.length === 0}{m.storage_search_status_done()}{:else if session.status === 'done'}{m.storage_search_status_partial()}{:else}{m.storage_search_status_error()}{/if} {#if session.status === 'running'}{/if} - {#if session.status === 'error'}{/if} + {#if failureMessage} + + {/if} {#if session.truncated}{/if} diff --git a/src/lib/components/storage/StorageSearchSessions.svelte b/src/lib/components/storage/StorageSearchSessions.svelte index fdce7564..8f93d6c5 100644 --- a/src/lib/components/storage/StorageSearchSessions.svelte +++ b/src/lib/components/storage/StorageSearchSessions.svelte @@ -34,8 +34,11 @@ class="loading loading-spinner loading-xs" aria-label={m.storage_search_status_running()} >{/if} - {#if session.status === 'done'}{m.storage_search_status_done()}{m.storage_search_status_done()}{/if} + {#if session.status === 'done' && session.failures.length > 0}{m.storage_search_status_partial()}{/if} {#if session.status === 'error'}{m.storage_search_status_error()} ({ goto: vi.fn(), invalidateAll: vi.fn() })); @@ -67,6 +68,29 @@ describe('StorageSearch', () => { ); }); + it('retains results and identifies buckets that fail during a multi-bucket search', async () => { + const { state, api } = createState(); + api.search.mockImplementation(({ bucket }: { bucket: string }) => { + if (bucket === 'beta') + return Promise.reject(new StorageError('access_denied', 'Access denied')); + return Promise.resolve({ + results: [{ key: 'report.csv', size: 1, lastModified: new Date(), isDirectory: false }], + truncated: false + }); + }); + render(StorageSearchWrapper, { state }); + await page.getByRole('button', { name: 'Open search' }).click(); + await page.getByLabelText('Search query').fill('report'); + await page.getByRole('button', { name: 'Search', exact: true }).click(); + + await expect + .element(page.getByRole('alert')) + .toHaveTextContent('Access to one or more selected buckets was denied.'); + await expect.element(page.getByRole('alert')).not.toHaveTextContent('beta'); + await expect.element(page.getByRole('button', { name: 'report.csv' })).toBeVisible(); + await expect.element(page.getByText('Partial', { exact: true })).toBeVisible(); + }); + it('adds a parallel session', async () => { const { state } = createState(); render(StorageSearchWrapper, { state, currentBucket: 'alpha' }); diff --git a/src/lib/server/storage/s3-provider-search.test.ts b/src/lib/server/storage/s3-provider-search.test.ts index 8e040f37..17edb01d 100644 --- a/src/lib/server/storage/s3-provider-search.test.ts +++ b/src/lib/server/storage/s3-provider-search.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { S3Client } from '@aws-sdk/client-s3'; +import { S3ServiceException } from '@aws-sdk/client-s3'; vi.mock('$lib/server/logging', () => ({ logger: { child: () => ({ trace: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() }) } @@ -98,6 +99,19 @@ describe('S3StorageProvider.search', () => { ({ provider, send } = makeProvider()); }); + it('maps an S3 access-denied error to HTTP 403', async () => { + send.mockRejectedValue( + new S3ServiceException({ + name: 'AccessDenied', + $fault: 'client', + $metadata: { httpStatusCode: 403 }, + message: 'Access denied' + }) + ); + + await expect(provider.search('report')).rejects.toMatchObject({ status: 403 }); + }); + it('matches case-insensitively anywhere in the full key path', async () => { send.mockResolvedValue({ Contents: [ diff --git a/src/lib/server/storage/s3-provider.ts b/src/lib/server/storage/s3-provider.ts index 50151f0a..2afabb10 100644 --- a/src/lib/server/storage/s3-provider.ts +++ b/src/lib/server/storage/s3-provider.ts @@ -481,44 +481,51 @@ export class S3StorageProvider implements StorageProvider { let scanned = 0; let truncated = false; - await this.listAllKeysProgressively( - prefix, - (batch) => { - for (const item of batch) { - if (signal?.aborted) { - throw new DOMException('The operation was aborted', 'AbortError'); - } - scanned++; - const relativeKey = item.key.slice(prefix.length); - if (maxDepth !== undefined && relativeKey.split('/').filter(Boolean).length > maxDepth) { - if (scanned >= maxKeysScanned) { - truncated = true; - return false; - } - continue; - } - const result = { - key: item.key, - size: item.size, - lastModified: item.lastModified ?? new Date(0), - isDirectory: item.key.endsWith('/') - }; - if (matches(result)) { - results.push(result); - onMatch?.(result); - if (results.length >= maxResults) { - truncated = true; - return false; + await withS3Errors( + () => + this.listAllKeysProgressively( + prefix, + (batch) => { + for (const item of batch) { + if (signal?.aborted) { + throw new DOMException('The operation was aborted', 'AbortError'); + } + scanned++; + const relativeKey = item.key.slice(prefix.length); + if ( + maxDepth !== undefined && + relativeKey.split('/').filter(Boolean).length > maxDepth + ) { + if (scanned >= maxKeysScanned) { + truncated = true; + return false; + } + continue; + } + const result = { + key: item.key, + size: item.size, + lastModified: item.lastModified ?? new Date(0), + isDirectory: item.key.endsWith('/') + }; + if (matches(result)) { + results.push(result); + onMatch?.(result); + if (results.length >= maxResults) { + truncated = true; + return false; + } + } + if (scanned >= maxKeysScanned) { + truncated = true; + return false; + } } - } - if (scanned >= maxKeysScanned) { - truncated = true; - return false; - } - } - return undefined; - }, - { signal } + return undefined; + }, + { signal } + ), + { bucket: this.bucket, operation: 'search' } ); log.info( diff --git a/src/lib/storage/search-stream.spec.ts b/src/lib/storage/search-stream.spec.ts index 954954aa..377ee7f5 100644 --- a/src/lib/storage/search-stream.spec.ts +++ b/src/lib/storage/search-stream.spec.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; import { readSearchStream } from './search-stream.js'; +import { StorageError } from './errors.js'; function createStream(events: object[]): ReadableStream { const encoder = new TextEncoder(); @@ -42,4 +43,10 @@ describe('readSearchStream', () => { readSearchStream(createStream([{ type: 'error', message: 'Failed' }])) ).rejects.toThrow('Failed'); }); + + it('preserves a streamed error classification', async () => { + await expect( + readSearchStream(createStream([{ type: 'error', code: 'access_denied', message: 'Denied' }])) + ).rejects.toMatchObject(new StorageError('access_denied', 'Denied')); + }); }); diff --git a/src/lib/storage/search-stream.ts b/src/lib/storage/search-stream.ts index 88ffc2b0..578b5c46 100644 --- a/src/lib/storage/search-stream.ts +++ b/src/lib/storage/search-stream.ts @@ -1,9 +1,11 @@ import type { SearchResultItem, StorageSearchResponse, StorageSearchUpdate } from './types.js'; +import { StorageError, type StorageErrorCode } from './errors.js'; interface SearchStreamEvent { type: 'batch' | 'snapshot' | 'complete' | 'error'; results?: SearchResultItem[]; truncated?: boolean; + code?: StorageErrorCode; message?: string; } @@ -41,7 +43,9 @@ export async function readSearchStream( for (const line of lines) { if (!line.trim()) continue; const event = JSON.parse(line) as SearchStreamEvent; - if (event.type === 'error') throw new Error(event.message ?? 'Search failed'); + if (event.type === 'error') { + throw new StorageError(event.code ?? 'unknown', event.message ?? 'Search failed'); + } if (event.type === 'batch') apply(event, false); if (event.type === 'snapshot' || event.type === 'complete') apply(event, true); } diff --git a/src/lib/storage/search.svelte.ts b/src/lib/storage/search.svelte.ts index b6a0d66f..8098d244 100644 --- a/src/lib/storage/search.svelte.ts +++ b/src/lib/storage/search.svelte.ts @@ -6,6 +6,7 @@ import { type SearchFilterField } from '$lib/storage/search-filter.js'; import type { StorageApi } from '$lib/storage/api.js'; +import { StorageError } from '$lib/storage/errors.js'; import type { RecentSearchEntry, SearchResultItem } from '$lib/storage/types.js'; export type SearchStatus = 'idle' | 'running' | 'done' | 'error'; @@ -14,6 +15,10 @@ export interface SearchResult extends SearchResultItem { bucket: string; } +export interface SearchFailure { + code: string; +} + export interface SearchSession { id: string; label: string; @@ -28,6 +33,7 @@ export interface SearchSession { status: SearchStatus; elapsed: number; truncated: boolean; + failures: SearchFailure[]; } export class StorageSearchState { @@ -47,6 +53,10 @@ export class StorageSearchState { active = $derived(this.sessions.find((session) => session.id === this.activeId)); runningCount = $derived(this.sessions.filter((session) => session.status === 'running').length); completedCount = $derived(this.sessions.filter((session) => session.status === 'done').length); + partialCount = $derived( + this.sessions.filter((session) => session.status === 'done' && session.failures.length > 0) + .length + ); constructor(options: { api: StorageApi; @@ -102,7 +112,8 @@ export class StorageSearchState { results: [], status: 'idle', elapsed: 0, - truncated: false + truncated: false, + failures: [] }); } @@ -218,12 +229,18 @@ export class StorageSearchState { this.controllers.get(id)?.abort(); const controller = new AbortController(); this.controllers.set(id, controller); - this.updateSession(id, { status: 'running', results: [], elapsed: 0, truncated: false }); + this.updateSession(id, { + status: 'running', + results: [], + elapsed: 0, + truncated: false, + failures: [] + }); const started = performance.now(); this.started.set(id, started); try { - const responses = await Promise.all( + const responses = await Promise.allSettled( buckets.map((bucket) => this.api .search({ @@ -243,16 +260,26 @@ export class StorageSearchState { ) ); if (controller.signal.aborted) return; - const results = responses.flatMap(({ bucket, results }) => + const successfulResponses = responses + .filter((response) => response.status === 'fulfilled') + .map((response) => response.value); + const failures: SearchFailure[] = responses.flatMap((response) => { + if (response.status !== 'rejected') return []; + return [ + { code: response.reason instanceof StorageError ? response.reason.code : 'unknown' } + ]; + }); + const results = successfulResponses.flatMap(({ bucket, results }) => results .filter((result) => this.matches(session, result)) .map((result) => ({ ...result, bucket })) ); this.updateSession(id, { - status: 'done', + status: successfulResponses.length > 0 ? 'done' : 'error', results, elapsed: Math.round(performance.now() - started), - truncated: responses.some((response) => response.truncated) + truncated: successfulResponses.some((response) => response.truncated), + failures }); } catch (error) { if (!(error instanceof DOMException && error.name === 'AbortError')) { @@ -293,7 +320,8 @@ export class StorageSearchState { results: [], status: 'idle', elapsed: 0, - truncated: false + truncated: false, + failures: [] }; } diff --git a/src/routes/(app)/api/storage/search/+server.ts b/src/routes/(app)/api/storage/search/+server.ts index 0bebb0d0..394e2851 100644 --- a/src/routes/(app)/api/storage/search/+server.ts +++ b/src/routes/(app)/api/storage/search/+server.ts @@ -1,4 +1,4 @@ -import { error } from '@sveltejs/kit'; +import { error, isHttpError } from '@sveltejs/kit'; import { createStorageProvider } from '$lib/server/storage/request-context.js'; import { SEARCH_DEFAULT_MAX_RESULTS, @@ -119,10 +119,20 @@ export const GET: RequestHandler = async (event) => { ); } catch (err) { storageSearchTotal.inc({ outcome: 'error', truncated: 'false' }); + const code = isHttpError(err) + ? err.status === 403 + ? 'access_denied' + : err.status === 404 + ? 'not_found' + : err.status >= 500 + ? 'server_error' + : 'unknown' + : 'unknown'; controller.enqueue( encoder.encode( JSON.stringify({ type: 'error', + code, message: err instanceof Error ? err.message : 'Search failed' }) + '\n' ) diff --git a/src/routes/(app)/api/storage/search/server.test.ts b/src/routes/(app)/api/storage/search/server.test.ts index 8ed206c9..5de6b7ef 100644 --- a/src/routes/(app)/api/storage/search/server.test.ts +++ b/src/routes/(app)/api/storage/search/server.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { error } from '@sveltejs/kit'; const { mockProvider, storageSearchTotal } = vi.hoisted(() => ({ mockProvider: { search: vi.fn(), listContainers: vi.fn() }, @@ -108,6 +109,20 @@ describe('GET /api/storage/search', () => { expect(storageSearchTotal.inc).toHaveBeenCalledWith({ outcome: 'error', truncated: 'false' }); }); + it('includes access-denied classification in streamed provider failures', async () => { + let accessDeniedError: unknown; + try { + error(403, 'Access denied'); + } catch (err) { + accessDeniedError = err; + } + mockProvider.search.mockRejectedValue(accessDeniedError); + + const response = await GET(mockEvent('bucket=documents&q=report')); + + await expect(response.text()).resolves.toContain('"code":"access_denied"'); + }); + it('rejects buckets outside the active connection', async () => { await expect(GET(mockEvent('bucket=private&q=report'))).rejects.toMatchObject({ status: 404 }); expect(mockProvider.search).not.toHaveBeenCalled(); From 9e02aec20cc62d29694add8018e8be113a0befdf Mon Sep 17 00:00:00 2001 From: F3l1x1vo Date: Thu, 3 Sep 2026 07:09:39 +0200 Subject: [PATCH 21/48] #180 add pagination to search --- e2e/storage/search.spec.ts | 32 +++++++++++ messages/de.json | 1 - messages/en.json | 1 - .../storage/StorageSearchResults.svelte | 55 +++++++++++++++++-- .../__tests__/StorageSearch.svelte.spec.ts | 12 ++-- src/lib/server/metrics.ts | 2 +- src/lib/server/storage/provider.ts | 15 +---- .../server/storage/s3-provider-search.test.ts | 40 ++++---------- src/lib/server/storage/s3-provider.ts | 29 +--------- src/lib/storage/api.spec.ts | 3 +- src/lib/storage/api.test-utils.ts | 2 +- src/lib/storage/search-stream.spec.ts | 5 +- src/lib/storage/search-stream.ts | 7 +-- src/lib/storage/search.svelte.ts | 5 -- src/lib/storage/state.svelte.spec.ts | 2 +- src/lib/storage/types.ts | 2 - .../(app)/api/storage/search/+server.ts | 22 +++----- .../(app)/api/storage/search/server.test.ts | 20 ++----- 18 files changed, 120 insertions(+), 135 deletions(-) diff --git a/e2e/storage/search.spec.ts b/e2e/storage/search.spec.ts index 63d8b0ca..195f9643 100644 --- a/e2e/storage/search.spec.ts +++ b/e2e/storage/search.spec.ts @@ -82,6 +82,38 @@ test.describe('Storage Search', () => { } }); + test('paginates search results', async ({ page }, testInfo) => { + const credentials = requireGarageCredentials(); + const client = createS3Client(credentials); + const prefix = uniquePrefix(testInfo, 'paginate-search'); + const keys: string[] = []; + + try { + for (let index = 1; index <= 30; index += 1) { + const key = `${prefix}report-${String(index).padStart(2, '0')}.txt`; + keys.push(key); + await putTextObject(client, credentials.bucket, key, `report ${index}`); + } + await connectToStorage(page, credentials); + + await page.getByRole('button', { name: 'Open search' }).first().click(); + await page.getByLabel('Search query').fill('report-'); + await page.getByRole('button', { name: 'Search', exact: true }).click(); + await page.getByLabel('Items per page').selectOption('25'); + + await expect(page.getByText('Page 1', { exact: true })).toBeVisible(); + await expect(page.getByRole('button', { name: 'report-01.txt' })).toBeVisible(); + await expect(page.getByRole('button', { name: 'report-26.txt' })).not.toBeVisible(); + + await page.getByRole('button', { name: 'Next page' }).click(); + await expect(page.getByText('Page 2', { exact: true })).toBeVisible(); + await expect(page.getByRole('button', { name: 'report-26.txt' })).toBeVisible(); + await expect(page.getByRole('button', { name: 'report-01.txt' })).not.toBeVisible(); + } finally { + await deleteKnownKeys(client, credentials.bucket, keys); + } + }); + test('filters and selects buckets via the bucket dropdown', async ({ page }, testInfo) => { const credentials = requireGarageCredentials(); const client = createS3Client(credentials); diff --git a/messages/de.json b/messages/de.json index 0999eaef..b0fec1ed 100644 --- a/messages/de.json +++ b/messages/de.json @@ -699,7 +699,6 @@ "storage_search_tab_search": "Suchen", "storage_search_tab_recent": "Letzte", "storage_search_no_results": "Keine Ergebnisse gefunden.", - "storage_search_truncated": "Es werden nur die ersten passenden Ergebnisse angezeigt.", "storage_search_failure_access_denied": "Der Zugriff auf einen oder mehrere ausgewählte Buckets wurde verweigert.", "storage_search_failure_generic": "Ein oder mehrere ausgewählte Buckets konnten nicht durchsucht werden. Bitte versuchen Sie es erneut.", "storage_search_session_label": "Suche {number}", diff --git a/messages/en.json b/messages/en.json index 13699a67..35c9847f 100644 --- a/messages/en.json +++ b/messages/en.json @@ -699,7 +699,6 @@ "storage_search_tab_search": "Search", "storage_search_tab_recent": "Recent", "storage_search_no_results": "No results found.", - "storage_search_truncated": "Only the first matching results are shown.", "storage_search_failure_access_denied": "Access to one or more selected buckets was denied.", "storage_search_failure_generic": "One or more selected buckets could not be searched. Please try again.", "storage_search_session_label": "Search {number}", diff --git a/src/lib/components/storage/StorageSearchResults.svelte b/src/lib/components/storage/StorageSearchResults.svelte index bbdf2abc..166de988 100644 --- a/src/lib/components/storage/StorageSearchResults.svelte +++ b/src/lib/components/storage/StorageSearchResults.svelte @@ -1,6 +1,8 @@ {#snippet SortIcon(key: SortKey)} @@ -114,8 +109,11 @@ {/snippet} {#if session.status !== 'idle'} -
    -
    +
    +
    {session.status === 'running' ? m.storage_search_searching() @@ -133,7 +131,9 @@ >{:else}{m.storage_search_status_error()}{/if}
    - {#if session.status === 'running'}{/if} {#if failureMessage} @@ -208,7 +208,7 @@ >{result.bucket}{result.isDirectory ? '-' : formatSize(result.size)}{result.isDirectory ? '-' : formatFileSize(result.size)}{result.lastModified.toLocaleDateString()} 1}
    From a8e1b1822b9170841708f213f31bcb9744e7bf04 Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Mon, 12 Oct 2026 16:01:47 +0200 Subject: [PATCH 46/48] fix(storage): share keep-alive agents across S3 clients A new S3Client with its own keep-alive agent was created per request, and its idle sockets were never closed or reused. Over time this exhausted file descriptors (the e2e Garage server crashed at ~1000 connections). Share pooled agents across clients and close idle sockets after 30 s. --- src/lib/server/storage/s3-client.test.ts | 25 ++++++++++++++++++++++++ src/lib/server/storage/s3-client.ts | 19 +++++++++++++----- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/lib/server/storage/s3-client.test.ts b/src/lib/server/storage/s3-client.test.ts index 8f2bae10..3a2f187d 100644 --- a/src/lib/server/storage/s3-client.test.ts +++ b/src/lib/server/storage/s3-client.test.ts @@ -11,7 +11,14 @@ vi.mock('@smithy/node-http-handler', () => ({ import { createS3Client } from './s3-client.js'; import { S3Client } from '@aws-sdk/client-s3'; +import { NodeHttpHandler } from '@smithy/node-http-handler'; +import type { Agent } from 'https'; const mockS3Client = vi.mocked(S3Client); +const mockHandler = vi.mocked(NodeHttpHandler); + +function handlerAgents(call: number) { + return mockHandler.mock.calls[call]![0] as { httpAgent: Agent; httpsAgent: Agent }; +} function baseConfig(overrides: Partial = {}): S3ConnectionConfig { return { @@ -24,6 +31,24 @@ function baseConfig(overrides: Partial = {}): S3ConnectionCo } describe('createS3Client', () => { + it('shares keep-alive agents across clients so idle connections are reused', () => { + mockHandler.mockClear(); + createS3Client(baseConfig({ tls: { verification: 'Full' } })); + createS3Client(baseConfig({ host: 'other.example.com', tls: { verification: 'Full' } })); + expect(handlerAgents(0).httpAgent).toBe(handlerAgents(1).httpAgent); + expect(handlerAgents(0).httpsAgent).toBe(handlerAgents(1).httpsAgent); + expect(handlerAgents(0).httpsAgent.options.keepAlive).toBe(true); + expect(handlerAgents(0).httpsAgent.options.rejectUnauthorized).not.toBe(false); + }); + + it('uses a separate unverified agent when TLS verification is disabled', () => { + mockHandler.mockClear(); + createS3Client(baseConfig({ tls: { verification: 'Full' } })); + createS3Client(baseConfig({ tls: { verification: 'None' } })); + expect(handlerAgents(1).httpsAgent).not.toBe(handlerAgents(0).httpsAgent); + expect(handlerAgents(1).httpsAgent.options.rejectUnauthorized).toBe(false); + }); + it('builds an http endpoint when tls is absent', () => { createS3Client(baseConfig({ tls: undefined })); expect(mockS3Client).toHaveBeenCalledWith( diff --git a/src/lib/server/storage/s3-client.ts b/src/lib/server/storage/s3-client.ts index db4a85fd..2abeeaa5 100644 --- a/src/lib/server/storage/s3-client.ts +++ b/src/lib/server/storage/s3-client.ts @@ -1,8 +1,18 @@ import { S3Client } from '@aws-sdk/client-s3'; import { NodeHttpHandler } from '@smithy/node-http-handler'; -import { Agent } from 'https'; +import { Agent as HttpAgent } from 'http'; +import { Agent as HttpsAgent } from 'https'; import type { S3ConnectionConfig } from './types.js'; +// A client is created per request. Sharing the agents lets those clients reuse +// pooled connections instead of each leaving its own idle keep-alive sockets +// open, which exhausted file descriptors on both the app and the S3 server. +// `timeout` only closes idle pooled sockets; active streams are unaffected. +const agentOptions = { keepAlive: true, maxFreeSockets: 64, timeout: 30_000 }; +const httpAgent = new HttpAgent(agentOptions); +const httpsAgent = new HttpsAgent(agentOptions); +const unverifiedHttpsAgent = new HttpsAgent({ ...agentOptions, rejectUnauthorized: false }); + function buildEndpointUrl(config: S3ConnectionConfig): string { const scheme = config.tls ? 'https' : 'http'; const portSuffix = config.port ? `:${config.port}` : ''; @@ -15,10 +25,9 @@ export function createS3Client(config: S3ConnectionConfig): S3Client { requestChecksumCalculation: 'WHEN_REQUIRED', endpoint: buildEndpointUrl(config), forcePathStyle: config.accessStyle === 'Path', - ...(config.tls?.verification === 'None' && { - requestHandler: new NodeHttpHandler({ - httpsAgent: new Agent({ rejectUnauthorized: false }) - }) + requestHandler: new NodeHttpHandler({ + httpAgent, + httpsAgent: config.tls?.verification === 'None' ? unverifiedHttpsAgent : httpsAgent }), ...(config.credentials && { credentials: { From c86d07d2f9a61a692cc21fc0827d2b5b7dd80d15 Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Tue, 13 Oct 2026 08:41:13 +0200 Subject: [PATCH 47/48] test(e2e): skip the folder rename test without Garage The test was placed after the closing of its describe block and missed the beforeEach that skips the suite when no Garage instance is configured, so it failed instead of being skipped. --- e2e/storage/file-operations.spec.ts | 77 +++++++++++++++-------------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/e2e/storage/file-operations.spec.ts b/e2e/storage/file-operations.spec.ts index dace442f..a78fd277 100644 --- a/e2e/storage/file-operations.spec.ts +++ b/e2e/storage/file-operations.spec.ts @@ -492,41 +492,44 @@ test.describe('Storage S3 — File Operations', () => { await deleteKnownKeys(client, credentials.bucket, cleanupKeys); } }); -}); -test('renames a folder subtree and removes its source marker', async ({ page }, testInfo) => { - const credentials = requireGarageCredentials(); - const client = createS3Client(credentials); - const prefix = uniquePrefix(testInfo, 'folder-rename'); - const oldRoot = `${prefix}docs/`; - const newRoot = `${prefix}renamed/`; - const cleanupKeys = [ - oldRoot, - `${oldRoot}one.txt`, - `${oldRoot}nested/two.txt`, - newRoot, - `${newRoot}one.txt`, - `${newRoot}nested/two.txt` - ]; - try { - await putDirectoryMarker(client, credentials.bucket, oldRoot); - await putTextObject(client, credentials.bucket, `${oldRoot}one.txt`, 'first'); - await putTextObject(client, credentials.bucket, `${oldRoot}nested/two.txt`, 'second'); - await connectAndOpenPrefix(page, credentials, prefix); - await rowByName(page, 'docs').click({ button: 'right' }); - await page.getByRole('menuitem', { name: 'Rename' }).click(); - await page.locator('.modal-box input').fill('renamed'); - await page.getByRole('button', { name: 'Rename', exact: true }).click(); - await expect - .poll(() => objectExists(client, credentials.bucket, `${newRoot}nested/two.txt`)) - .toBe(true); - expect(await getObjectText(client, credentials.bucket, `${newRoot}one.txt`)).toBe('first'); - expect(await getObjectText(client, credentials.bucket, `${newRoot}nested/two.txt`)).toBe( - 'second' - ); - expect(await objectExists(client, credentials.bucket, oldRoot)).toBe(false); - expect(await objectExists(client, credentials.bucket, `${oldRoot}one.txt`)).toBe(false); - expect(await objectExists(client, credentials.bucket, `${oldRoot}nested/two.txt`)).toBe(false); - } finally { - await deleteKnownKeys(client, credentials.bucket, cleanupKeys); - } + + test('renames a folder subtree and removes its source marker', async ({ page }, testInfo) => { + const credentials = requireGarageCredentials(); + const client = createS3Client(credentials); + const prefix = uniquePrefix(testInfo, 'folder-rename'); + const oldRoot = `${prefix}docs/`; + const newRoot = `${prefix}renamed/`; + const cleanupKeys = [ + oldRoot, + `${oldRoot}one.txt`, + `${oldRoot}nested/two.txt`, + newRoot, + `${newRoot}one.txt`, + `${newRoot}nested/two.txt` + ]; + try { + await putDirectoryMarker(client, credentials.bucket, oldRoot); + await putTextObject(client, credentials.bucket, `${oldRoot}one.txt`, 'first'); + await putTextObject(client, credentials.bucket, `${oldRoot}nested/two.txt`, 'second'); + await connectAndOpenPrefix(page, credentials, prefix); + await rowByName(page, 'docs').click({ button: 'right' }); + await page.getByRole('menuitem', { name: 'Rename' }).click(); + await page.locator('.modal-box input').fill('renamed'); + await page.getByRole('button', { name: 'Rename', exact: true }).click(); + await expect + .poll(() => objectExists(client, credentials.bucket, `${newRoot}nested/two.txt`)) + .toBe(true); + expect(await getObjectText(client, credentials.bucket, `${newRoot}one.txt`)).toBe('first'); + expect(await getObjectText(client, credentials.bucket, `${newRoot}nested/two.txt`)).toBe( + 'second' + ); + expect(await objectExists(client, credentials.bucket, oldRoot)).toBe(false); + expect(await objectExists(client, credentials.bucket, `${oldRoot}one.txt`)).toBe(false); + expect(await objectExists(client, credentials.bucket, `${oldRoot}nested/two.txt`)).toBe( + false + ); + } finally { + await deleteKnownKeys(client, credentials.bucket, cleanupKeys); + } + }); }); From 7d3321a89e3c9475bd3ed5b82d74751ef1c8e4ae Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Tue, 13 Oct 2026 08:41:35 +0200 Subject: [PATCH 48/48] fix(storage): sanitise archive entry names Object keys are untrusted: anyone with write access to a shared bucket can create keys such as `../../.bashrc` or `/etc/profile`, which naive extractors would write outside the target directory. Name ZIP entries relative to the download prefix and drop empty, `.` and `..` segments, leading slashes, backslashes and drive letters. Files that collide after sanitising get a numeric suffix. --- src/lib/server/storage/download-manifests.ts | 19 +++-- src/lib/server/storage/zip-stream.test.ts | 77 +++++++++++++++++--- src/lib/server/storage/zip-stream.ts | 74 ++++++++++++++++--- 3 files changed, 144 insertions(+), 26 deletions(-) diff --git a/src/lib/server/storage/download-manifests.ts b/src/lib/server/storage/download-manifests.ts index f1735da8..74937ecb 100644 --- a/src/lib/server/storage/download-manifests.ts +++ b/src/lib/server/storage/download-manifests.ts @@ -103,12 +103,17 @@ async function expandKeys(provider: StorageProvider, keys: string[]): Promise { ); const object = new ReadableStream({ pull, cancel }, { highWaterMark: 0 }); const getObject = vi.fn(async () => ({ stream: object })); - const reader = createZipStream({ getObject } as unknown as StorageProvider, [ - { key: 'large.bin', size: 100_000, isDirectory: false } - ]).getReader(); + const reader = createZipStream( + { getObject } as unknown as StorageProvider, + [{ key: 'large.bin', size: 100_000, isDirectory: false }], + '' + ).getReader(); await new Promise((resolve) => setTimeout(resolve, 10)); expect(getObject).not.toHaveBeenCalled(); await reader.read(); // Local header. @@ -40,8 +42,8 @@ describe('createZipStream', () => { getObject: async () => ({ stream: new Response('abc').body! }) } as unknown as StorageProvider; - const stream = createZipStream(provider, entries); - expect(zipStreamSize(entries)).toBe((await new Response(stream).arrayBuffer()).byteLength); + const stream = createZipStream(provider, entries, ''); + expect(zipStreamSize(entries, '')).toBe((await new Response(stream).arrayBuffer()).byteLength); }); it('streams readable, uncompressed entries without staging object contents', async () => { const objects = new Map([ @@ -54,11 +56,15 @@ describe('createZipStream', () => { const archive = new Uint8Array( await new Response( - createZipStream(provider, [ - { key: 'reports/', size: 0, isDirectory: true }, - { key: 'reports/one.txt', size: 12, isDirectory: false }, - { key: 'reports/two.txt', size: 13, isDirectory: false } - ]) + createZipStream( + provider, + [ + { key: 'reports/', size: 0, isDirectory: true }, + { key: 'reports/one.txt', size: 12, isDirectory: false }, + { key: 'reports/two.txt', size: 13, isDirectory: false } + ], + '' + ) ).arrayBuffer() ); const zip = new AdmZip(Buffer.from(archive)); @@ -80,7 +86,7 @@ describe('createZipStream', () => { // Central headers 46 + 1 + extra: sizes only (4 + 16), then sizes and offset (4 + 16 + 8). const centralDirectory = 46 + 1 + 20 + (46 + 1 + 28); // ZIP64 end record, locator and classic end record. - expect(zipStreamSize(entries)).toBe(dataEnd + centralDirectory + 98); + expect(zipStreamSize(entries, '')).toBe(dataEnd + centralDirectory + 98); }); it('uses ZIP64 records when the central directory has 65,535 entries', async () => { const provider = {} as StorageProvider; @@ -90,7 +96,7 @@ describe('createZipStream', () => { isDirectory: true })); const archive = new Uint8Array( - await new Response(createZipStream(provider, entries)).arrayBuffer() + await new Response(createZipStream(provider, entries, '')).arrayBuffer() ); const view = new DataView(archive.buffer, archive.byteOffset, archive.byteLength); const eocdOffset = archive.length - 22; @@ -99,4 +105,51 @@ describe('createZipStream', () => { expect(view.getUint16(eocdOffset + 8, true)).toBe(0xffff); expect(view.getUint32(eocdOffset - 20, true)).toBe(0x07064b50); }, 60_000); + + it('names entries relative to the download prefix', async () => { + const provider = { + getObject: async () => ({ stream: objectStream('abc'), contentLength: 3 }) + } as unknown as StorageProvider; + const entries = [ + { key: 'data/reports/', size: 0, isDirectory: true }, + { key: 'data/reports/one.txt', size: 3, isDirectory: false } + ]; + const archive = await new Response(createZipStream(provider, entries, 'data/')).arrayBuffer(); + const zip = new AdmZip(Buffer.from(archive)); + + expect(zip.getEntries().map((entry) => entry.entryName)).toEqual([ + 'reports/', + 'reports/one.txt' + ]); + expect(zipStreamSize(entries, 'data/')).toBe(archive.byteLength); + }); + + it('never emits entry names that resolve outside the extraction directory', async () => { + const provider = { + getObject: async () => ({ stream: objectStream('x'), contentLength: 1 }) + } as unknown as StorageProvider; + const entries = [ + { key: '../', size: 0, isDirectory: true }, + { key: '../../.bashrc', size: 1, isDirectory: false }, + { key: '/etc/profile', size: 1, isDirectory: false }, + { key: '..\\..\\evil.txt', size: 1, isDirectory: false }, + { key: 'C:/Windows/evil.txt', size: 1, isDirectory: false }, + { key: 'a/./b/../c.txt', size: 1, isDirectory: false }, + { key: 'c.txt', size: 1, isDirectory: false }, + { key: 'a/b/c.txt', size: 1, isDirectory: false } + ]; + const archive = await new Response(createZipStream(provider, entries, '')).arrayBuffer(); + const names = new AdmZip(Buffer.from(archive)).getEntries().map((entry) => entry.entryName); + + expect(names).toEqual([ + '.bashrc', + 'etc/profile', + '.._.._evil.txt', + 'C_/Windows/evil.txt', + 'a/b/c.txt', + 'c.txt', + 'a/b/c (2).txt' + ]); + expect(zipStreamSize(entries, '')).toBe(archive.byteLength); + }); }); diff --git a/src/lib/server/storage/zip-stream.ts b/src/lib/server/storage/zip-stream.ts index 79d23b43..a345a764 100644 --- a/src/lib/server/storage/zip-stream.ts +++ b/src/lib/server/storage/zip-stream.ts @@ -26,13 +26,13 @@ export interface ZipEntry { * Record lengths come from the same builders the stream uses, so the * advertised Content-Length cannot drift from the generated bytes. */ -export function zipStreamSize(entries: ZipEntry[]): number { +export function zipStreamSize(entries: ZipEntry[], prefix: string): number { let offset = 0; let centralDirectorySize = 0; + const layout = archiveLayout(entries, prefix); - for (const entry of entries) { + for (const { entry, name } of layout) { const localOffset = offset; - const name = entryName(entry); const zip64 = entry.size >= UINT32_MAX; offset += localHeader(name, zip64).length; offset += entry.size + dataDescriptor(0, entry.size, zip64).length; @@ -49,7 +49,7 @@ export function zipStreamSize(entries: ZipEntry[]): number { return ( offset + centralDirectorySize + - endOfCentralDirectory(entries.length, centralDirectorySize, offset).length + endOfCentralDirectory(layout.length, centralDirectorySize, offset).length ); } @@ -211,8 +211,61 @@ function endOfCentralDirectory(count: number, size: number, offset: number): Uin ); } -function entryName(entry: ZipEntry): Uint8Array { - return bytes(entry.isDirectory && !entry.key.endsWith('/') ? `${entry.key}/` : entry.key); +/** + * Object keys are untrusted: anyone with write access to the bucket can create + * keys such as `../../.bashrc` or `/etc/profile`. Names are made relative to + * the folder the download started from and reduced to plain path segments so + * that no extractor can resolve them outside its target directory. + */ +function sanitiseEntryName(key: string, prefix: string): string { + const base = prefix && !prefix.endsWith('/') ? `${prefix}/` : prefix; + const relative = base && key.startsWith(base) ? key.slice(base.length) : key; + const segments = relative + .split('/') + .map((segment) => segment.replaceAll('\\', '_')) + .filter((segment) => segment !== '' && segment !== '.' && segment !== '..'); + // A leading drive letter (`C:`) is treated as an absolute path on Windows. + if (segments.length > 0 && /^[a-z]:/i.test(segments[0])) { + segments[0] = `${segments[0][0]}_${segments[0].slice(2)}`; + } + return segments.join('/'); +} + +function uniqueName(name: string, used: Set): string { + if (!used.has(name)) return name; + const slash = name.lastIndexOf('/'); + const dot = name.lastIndexOf('.'); + const extensionStart = dot > slash + 1 ? dot : name.length; + for (let index = 2; ; index++) { + const candidate = `${name.slice(0, extensionStart)} (${index})${name.slice(extensionStart)}`; + if (!used.has(candidate)) return candidate; + } +} + +/** + * Resolve the archive name of every entry. Directories that collapse to an + * empty or already-present name are omitted; files that collide after + * sanitising get a numeric suffix so that no entry overwrites another. + */ +function archiveLayout( + entries: ZipEntry[], + prefix: string +): Array<{ entry: ZipEntry; name: Uint8Array }> { + const used = new Set(); + const layout: Array<{ entry: ZipEntry; name: Uint8Array }> = []; + for (const entry of entries) { + const sanitised = sanitiseEntryName(entry.key, prefix); + let name: string; + if (entry.isDirectory) { + if (!sanitised || used.has(`${sanitised}/`)) continue; + name = `${sanitised}/`; + } else { + name = uniqueName(sanitised || 'unnamed', used); + } + used.add(name); + layout.push({ entry, name: bytes(name) }); + } + return layout; } /** @@ -220,7 +273,11 @@ function entryName(entry: ZipEntry): Uint8Array { * forwarded directly from the provider; only central-directory metadata stays * in memory. */ -export function createZipStream(provider: StorageProvider, entries: ZipEntry[]): ReadableStream { +export function createZipStream( + provider: StorageProvider, + entries: ZipEntry[], + prefix: string +): ReadableStream { let cancelled = false; let activeReader: ReadableStreamDefaultReader | undefined; @@ -228,9 +285,8 @@ export function createZipStream(provider: StorageProvider, entries: ZipEntry[]): let offset = 0; const centralEntries: CentralDirectoryEntry[] = []; try { - for (const entry of entries) { + for (const { entry, name } of archiveLayout(entries, prefix)) { if (cancelled) return; - const name = entryName(entry); if (name.length > UINT16_MAX) throw new Error('Archive entry name is too long'); const sizeZip64 = entry.size >= UINT32_MAX; yield localHeader(name, sizeZip64);