chore: Merge upstream 2.54.0#86
Conversation
94% of minimum 50% translated source file: 'frontend/src/i18n/en.json' on 'es'. Sync of partially translated files: untranslated content is included with an empty translation or source language content depending on file format
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
100% translated source file: 'frontend/src/i18n/en.json' on 'no'.
Co-authored-by: Henrique Dias <mail@hacdias.com>
…nity MB/s (filebrowser#5456) Co-authored-by: Henrique Dias <mail@hacdias.com>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: Ryan Miller <ryan.miller@infinitetactics.com>
Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 6.3.6 to 6.4.1. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/create-vite@6.4.1/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 6.4.1 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Henrique Dias <mail@hacdias.com>
…lebrowser#5637) Co-authored-by: FadedAtlas <fadedatlas.shield181@slmail.me>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughThis pull request introduces significant architectural and feature changes including CI/CD workflow restructuring (GitHub Actions), build system migration from Make to Task, widespread package renamings (http→fbhttp, errors→fberrors), major configuration system refactoring with Viper-based setup, new frontend features (CSV viewer, context menu, logout page handling), dependency upgrades (Go 1.24→1.25, frontend packages), removal of legacy systems (upgrade command, importer, tests), and substantial documentation reorganization. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
CONTRIBUTING.md (1)
102-102: Minor typo: double comma.There's a double comma in the Translations section.
📝 Suggested fix
-Translations are managed on Transifex, which is an online website where everyone can contribute and translate strings for our project. It automatically syncs with the main language file \(in English\) and,, for you to contribute, you just need to: +Translations are managed on Transifex, which is an online website where everyone can contribute and translate strings for our project. It automatically syncs with the main language file \(in English\) and, for you to contribute, you just need to:files/file.go (1)
450-471: Duplicate image resolution calculation.For image files, the resolution is calculated twice:
- Lines 450-457: Pre-calculates resolution based on MIME type
- Line 467:
detectTypeis called withcalcImgRes=true, which calculates resolution again at lines 256-262This opens and decodes each image file twice, impacting performance for directories with many images.
Suggested fix: Remove pre-calculation, let detectType handle it
- if !file.IsDir && strings.HasPrefix(mime.TypeByExtension(file.Extension), "image/") && calcImgRes { - resolution, err := calculateImageResolution(file.Fs, file.Path) - if err != nil { - log.Printf("Error calculating resolution for image %s: %v", file.Path, err) - } else { - file.Resolution = resolution - } - } - if file.IsDir { listing.NumDirs++ } else {frontend/src/i18n/index.ts (1)
61-93: Unreachable locale detection cases due to ordering.The switch-case pattern matching has duplicate locale handlers where broader patterns precede narrower ones, making the latter unreachable:
/^ru.*/i(line 61) matches before/^ru\b/(line 76)/^tr.*/i(line 64) matches before/^tr\b/(line 88)/^uk.*/i(line 67) matches before/^uk\b/(line 91)Since the
switch(true)evaluates cases top-to-bottom, the narrower patterns will never be reached. Either remove the duplicates or consolidate the logic.Proposed fix - Remove duplicate cases
case /^uk.*/i.test(locale): locale = "uk_UA"; break; - case /^de\b/.test(locale): - locale = "de"; - break; - case /^ro\b/.test(locale): - locale = "ro"; - break; - case /^ru\b/.test(locale): - locale = "ru"; - break; - case /^pl\b/.test(locale): - locale = "pl"; - break; - case /^ko\b/.test(locale): - locale = "ko"; - break; - case /^sk\b/.test(locale): - locale = "sk"; - break; - case /^tr\b/.test(locale): - locale = "tr"; - break; - case /^uk\b/.test(locale): - locale = "uk"; - break; - + case /^de\b/.test(locale): + locale = "de"; + break; + case /^ro\b/.test(locale): + locale = "ro"; + break; + case /^pl\b/.test(locale): + locale = "pl"; + break; + case /^ko\b/.test(locale): + locale = "ko"; + break; + case /^sk\b/.test(locale): + locale = "sk"; + break; case /^vi\b/.test(locale):Note: Keep only the first occurrence of ru, tr, and uk cases (lines 61-68) and remove the duplicate narrower patterns.
cmd/users_import.go (1)
54-71: Bug: Backup saves new users instead of existing users.When
replaceis true, the intent is to back up existing users before deletion. However, line 60 marshalslist(the new users being imported) instead ofoldUsers(the existing users). This defeats the purpose of the backup.Suggested fix
if replace { oldUsers, userImportErr := st.Users.Gets("") if userImportErr != nil { return userImportErr } - err = marshal("users.backup.json", list) + err = marshal("users.backup.json", oldUsers) if err != nil { return err }frontend/src/types/user.d.ts (1)
22-36: AddaceEditorThemetoIUserForminterface.
aceEditorThemeis editable via the user settings form (Profile.vue) and is submitted to the API as part of user profile updates, but is not defined in theIUserForminterface despite being present inIUser. AddaceEditorTheme?: string;to maintain type consistency.frontend/src/views/settings/Profile.vue (1)
164-171: Validation may block password updates whencurrentPasswordis not required.When
isCurrentPasswordRequiredisfalse(non-JSON auth methods),currentPassword.valueremains an empty string, causing the validation at line 167 to fail and block all password updates.Consider making this check conditional:
🐛 Proposed fix
if ( password.value !== passwordConf.value || password.value === "" || - currentPassword.value === "" || + (isCurrentPasswordRequired.value && currentPassword.value === "") || authStore.user === null ) { return; }cmd/users.go (1)
29-50: Header and data columns are misaligned.The header on line 30 has 15 columns, but the
fmt.Fprintfon line 33 outputs 16 values (the newRedirectAfterCopyMovefield was added without updating the header). This will cause all columns afterS.Clickto be misaligned in the output.Suggested fix - add missing header column
- fmt.Fprintln(w, "ID\tUsername\tScope\tLocale\tV. Mode\tS.Click\tAdmin\tExecute\tCreate\tRename\tModify\tDelete\tShare\tDownload\tPwd Lock") + fmt.Fprintln(w, "ID\tUsername\tScope\tLocale\tV. Mode\tS.Click\tRedirect\tAdmin\tExecute\tCreate\tRename\tModify\tDelete\tShare\tDownload\tPwd Lock")cmd/root.go (1)
483-488: Typo and consider returning error instead oflog.Fatal.Line 483 has a typo: "initialize wth" should be "initialized with".
Also, using
log.Fatalon line 487 prevents proper error propagation up the call stack. Consider returning an error instead to allow the caller to handle cleanup appropriately.Suggested fix
} else { - log.Printf("User '%s' initialize wth user-provided password\n", username) + log.Printf("User '%s' initialized with user-provided password\n", username) } if username == "" || password == "" { - log.Fatal("username and password cannot be empty during quick setup") + return errors.New("username and password cannot be empty during quick setup") }
🤖 Fix all issues with AI agents
In `@cmd/rule_rm.go`:
- Around line 56-64: The closures user and global perform unsafe slice removals
using u.Rules[:i] and u.Rules[f+1:] without validating i and f; add defensive
bounds checks at the start of each closure to ensure i and f are within 0 <= i
<= f < len(u.Rules) (and similarly for len(s.Rules)) and return a descriptive
error instead of proceeding if the indices are invalid; keep the calls to
st.Users.Save(u) and st.Settings.Save(s) unchanged but only call them after
validation.
In `@frontend/src/api/pub.ts`:
- Around line 43-45: The single-file branch appends files[0] raw to the URL,
which breaks for spaces/special chars; change it to append an encoded path
segment (e.g., use encodeURIComponent(files[0])) instead of files[0] so the URL
is properly escaped (keep the trailing "?" as before) — mirror the encoding
approach used in the multi-file branch that encodes arg.
In `@frontend/src/api/search.ts`:
- Around line 55-64: The loop in frontend/src/api/search.ts parses each line
into item and then checks item.isDir, but the backend sends the boolean as
item.dir; update the fallback handling so it uses the correct property (e.g.,
replace the check if (item.isDir) with if (item.dir) or normalize the object by
setting item.isDir = item.dir right after parsing) before appending "/" and
invoking callback(item).
- Around line 41-45: The code parses each line into a ResourceItem but checks
item.isDir while the backend sends "dir", so directory URLs never get a trailing
slash; update the parsing/logic in frontend/src/api/search.ts to use the backend
property (use item.dir) or explicitly map it (e.g., set item.isDir = item.dir
after JSON.parse) and then use that mapped value when appending the "/" to
item.url; ensure the ResourceItem type/signature is updated accordingly so
future checks refer to the correct property name (item.dir or item.isDir
consistently).
In `@frontend/src/components/prompts/UploadFiles.vue`:
- Around line 114-124: The throttling wrapper causes mismatched deltas because
calculateSpeed uses oldSentBytes from the watch while elapsedTime is measured
from lastSpeedUpdate; fix by introducing and maintaining a separate
lastSentBytes variable that is updated only when calculateSpeed actually runs:
add a module-level lastSentBytes, use it inside calculateSpeed to compute
bytesSinceLastCalc = sentBytes - lastSentBytes, update lastSentBytes and
lastSpeedUpdate at the end of calculateSpeed, and have throttledCalculateSpeed
call calculateSpeed(sentBytes) (or pass current sentBytes) instead of relying on
oldSentBytes so throttled calls compute correct speed.
In `@frontend/src/views/files/Editor.vue`:
- Around line 156-159: The saveAction currently calls save() then always calls
next(), so navigation proceeds even if the save fails; change saveAction to
await save(true) (to throw on failure) and only call next() when that await
succeeds, i.e. wrap the await save(true) in a try/catch or let the thrown error
prevent calling next(); reference saveAction, save(), and next() when updating
the callback so failed saves block navigation just like close() does.
In `@http/search.go`:
- Around line 25-57: The timer is created once (timeout := time.NewTimer(...))
but never reset so timeout.C only fires once; fix by resetting or replacing the
timer: either swap to a time.NewTicker and use ticker.C (stopping it on return),
or, if keeping the timer, call timeout.Reset(searchPingInterval * time.Second)
inside the timeout.C case (right after setting infoBytes = nil) so the heartbeat
continues each interval; keep the existing defer timeout.Stop() (or stop the
ticker) and leave the rest of the loop logic (response, ctx, cancel, w, flusher)
unchanged.
In `@www/docs/command-execution.md`:
- Around line 51-54: The Docker example is invalid because the base image "FROM
filebrowser/filebrowser" is BusyBox-based and lacks package managers; replace
that line by either building FROM a full distro (e.g., Ubuntu/Debian) and
install p7zip-full with apt and then install or copy the filebrowser binary, or
switch to the s6 variant "FROM filebrowser/filebrowser:s6" (Alpine-based) and
install p7zip via apk; update the document to show one of these two valid
approaches and remove the broken "RUN sudo apt install p7zip-full" example.
🟡 Minor comments (30)
frontend/test-results/.last-run.json-1-4 (1)
1-4: Addtest-resultsdirectory to.gitignore.The
frontend/test-results/.last-run.jsonfile is a test artifact that should not be committed to version control. The frontend project currently has no test framework configured (no test scripts inpackage.jsonand no testing dependencies), making this directory extraneous. Additionally, the file's"status": "failed"with an emptyfailedTestsarray suggests an incomplete or abandoned test run.Add
test-results/to the root.gitignoreto prevent future test artifacts from being committed.www/docs/cli/filebrowser-config-import.md-7-12 (1)
7-12: Fix typos: "unexisting" and "nonexisting" should be "nonexistent".The words "unexisting" (line 8) and "nonexisting" (line 10) are non-standard. The correct term is "nonexistent".
📝 Proposed fix
Import a configuration file. This will replace all the existing -configuration. Can be used with or without unexisting databases. +configuration. It can be used with or without nonexistent databases. -If used with a nonexisting database, a key will be generated +If used with a nonexistent database, a key will be generated automatically. Otherwise the key will be kept the same as in the database.frontend/src/css/context-menu.css-11-17 (1)
11-17: Duplicatedisplayproperty will cause unexpected behavior.Line 12 sets
display: blockwhich is immediately overridden bydisplay: flexon line 15. The first declaration is redundant and should be removed.Proposed fix
.context-menu .action { - display: block; width: 100%; border-radius: 0; display: flex; align-items: center; }frontend/src/stores/auth.ts-10-14 (1)
10-14: Clear the logout timer inclearUser()to prevent stale timeout execution.The
logoutTimerstate stores a timeout handle, but whenclearUser()callsthis.$reset()during logout, it only resets the state reference without canceling the pending timeout. TheclearTimeout()call exists inparseToken()to clear old timers before setting new ones, but it's missing during logout. If a timeout is scheduled and logout is triggered, the timeout will still fire after the user is cleared, potentially causing unexpected behavior.♻️ Suggested fix
// easily reset state using `$reset` clearUser() { + if (this.logoutTimer !== null) { + clearTimeout(this.logoutTimer); + } this.$reset(); },frontend/src/i18n/hr.json-46-46 (1)
46-46: Untranslated strings in Croatian localization file.Several entries remain in English and should be translated to Croatian:
- Line 46:
"stopSearch": "Stop searching"→ should be Croatian (e.g., "Zaustavi pretraživanje")- Line 175:
"aceEditorTheme": "Ace editor theme"→ should be Croatian (e.g., "Tema Ace uređivača")- Line 262:
"currentPassword": "Your Current Password"→ should be Croatian (e.g., "Vaša trenutna lozinka")Also applies to: 175-175, 262-262
frontend/src/api/utils.ts-95-111 (1)
95-111: Potential issue: returned timeout ID cannot cancel chained timeouts.The function returns the initial timeout ID, but when
remaining > MAX_DELAY, subsequent scheduled timeouts receive new IDs. If a caller usesclearTimeoutwith the returned ID after the first chunk has elapsed, the remaining chain will continue executing.If cancellation is a requirement for the logout timer use case, consider returning a cancel function or tracking the latest timeout ID:
💡 Suggested fix with cancellation support
-export function setSafeTimeout(callback: () => void, delay: number): number { +export function setSafeTimeout(callback: () => void, delay: number): () => void { const MAX_DELAY = 86_400_000; let remaining = delay; + let timeoutId: number; function scheduleNext(): number { if (remaining <= MAX_DELAY) { - return window.setTimeout(callback, remaining); + timeoutId = window.setTimeout(callback, remaining); + return timeoutId; } else { - return window.setTimeout(() => { + timeoutId = window.setTimeout(() => { remaining -= MAX_DELAY; scheduleNext(); }, MAX_DELAY); + return timeoutId; } } - return scheduleNext(); + scheduleNext(); + return () => window.clearTimeout(timeoutId); }frontend/src/i18n/bg.json-46-50 (1)
46-50: Several strings remain untranslated.The following keys are still in English and should be translated to Bulgarian for consistency:
buttons.stopSearch,buttons.editAsText,buttons.increaseFontSize,buttons.decreaseFontSizefiles.csvTooLarge,files.csvLoadFailed,files.showingRows,files.columnSeparator,files.csvSeparators.*login.passwordTooShortsettings.currentPasswordThis can be addressed in a follow-up if translations are pending.
Also applies to: 83-91, 118-118, 262-262
frontend/src/components/settings/AceEditorTheme.vue-25-27 (1)
25-27: Incorrect type cast for event target.
SelectHTMLAttributesis a Vue type for component props, not for DOM element access. The cast should useHTMLSelectElementto properly type the event target.Suggested fix
-import { type SelectHTMLAttributes } from "vue"; import { themes } from "ace-builds/src-noconflict/ext-themelist"; // ... const change = (event: Event) => { - emit("update:aceEditorTheme", (event.target as SelectHTMLAttributes)?.value); + emit("update:aceEditorTheme", (event.target as HTMLSelectElement)?.value); };frontend/src/i18n/lv.json-46-46 (1)
46-46: Untranslated strings detected.Several strings remain in English and should be translated to Latvian:
- Line 46:
"stopSearch": "Stop searching"→ should be"Pārtraukt meklēšanu"- Line 177:
"administrator": "Administrator"→ should be"Administrators"- Line 262:
"currentPassword": "Your Current Password"→ should be"Jūsu pašreizējā parole"Also applies to: 177-177, 262-262
frontend/src/components/ContextMenu.vue-22-27 (1)
22-27: Computedleftmay use staleclientWidthon initial render.When
showtransitions totrue, this computed runs before the element is actually rendered, socontextMenu.value?.clientWidthwill likely be0orundefined. The menu position won't account for its actual width on the first render.Consider using
nextTickor aResizeObserverto recalculate position after the element is visible.Suggested approach using nextTick
+import { ref, watch, computed, onUnmounted, nextTick } from "vue"; + +const left = ref(0); + +const calculateLeft = () => { + nextTick(() => { + left.value = Math.min( + props.pos.x, + window.innerWidth - (contextMenu.value?.clientWidth ?? 0) + ); + }); +}; + +watch( + () => [props.show, props.pos.x], + ([show]) => { + if (show) { + calculateLeft(); + } + }, + { immediate: true } +);frontend/src/utils/auth.ts-31-37 (1)
31-37: Guard against missing or expiredexpclaim.The non-null assertion on
data.exp!could cause issues if the JWT lacks anexpclaim (resulting inNaNtimeout). Additionally, an already-expired token would produce a negative timeout.Suggested fix
- const expiresAt = new Date(data.exp! * 1000); - const timeout = expiresAt.getTime() - Date.now(); + if (!data.exp) { + console.warn("JWT missing exp claim, idle timeout disabled"); + return; + } + const expiresAt = new Date(data.exp * 1000); + const timeout = expiresAt.getTime() - Date.now(); + if (timeout <= 0) { + logout("expired"); + return; + }frontend/src/i18n/ru_RU.json-284-284 (1)
284-284: Untranslated string in Russian locale.The
currentPasswordvalue is in English ("Your Current Password") but should be translated to Russian.Suggested fix
- "currentPassword": "Your Current Password" + "currentPassword": "Ваш текущий пароль"frontend/src/i18n/ru_RU.json-50-50 (1)
50-50: Untranslated string in Russian locale.The
stopSearchvalue is in English ("Stop searching") but should be translated to Russian for consistency with the rest of the locale file.Suggested fix
- "stopSearch": "Stop searching", + "stopSearch": "Остановить поиск",www/docs/cli/filebrowser-config-init.md-8-11 (1)
8-11: Minor grammar issue: hyphenate "user-related".When used as a compound adjective before a noun, "user related" should be hyphenated.
📝 Suggested fix
-to the defaults when creating new users and you don't +to the defaults when creating new users and you don'tChange line 9 from:
'filebrowser config set'. The user related flags applyto:
'filebrowser config set'. The user-related flags applywww/docs/authentication.md-43-43 (1)
43-43: Incorrect heading level for "No Authentication" section."No Authentication" is described as the third authentication method (alongside JSON Auth and Proxy Header), but it's rendered as a subsection (
###) under "Proxy Header" instead of a top-level section (##).Proposed fix
-### No Authentication +## No Authenticationcmd/config.go-316-319 (1)
316-319: Error fromGetStringis silently ignored when it fails.If
flags.GetString(flag.Name)returns an error, the error is only used in theifcondition but not appended to theerrsslice. Theerrvariable inside theifblock shadows the outererr, so a failure here goes unreported.Proposed fix
case "hidden-files": - if hiddenFileString, err := flags.GetString(flag.Name); err == nil { + hiddenFileString, err := flags.GetString(flag.Name) + if err == nil { ser.HiddenFiles = convertFileStrToFileMap(hiddenFileString) }.github/workflows/docs.yml-5-7 (1)
5-7: Path patterns may not trigger as expected.The path
'www'only matches a file namedwww, not files within thewww/directory. Use'www/**'to match all files recursively.Suggested fix
pull_request: paths: - - 'www' + - 'www/**' - '*.md'.github/workflows/docs.yml-51-52 (1)
51-52: Missing stepidcauses undefinedsteps.deploymentreference.Line 35 references
steps.deployment.outputs.page_url, but the deploy step lacks anid: deploymentattribute. The environment URL won't be set correctly.Suggested fix
- name: Deploy to GitHub Pages + id: deployment uses: actions/deploy-pages@v4frontend/src/i18n/lv_LV.json-46-46 (1)
46-46: Untranslated strings remain in English.Two strings are still in English and should be translated to Latvian for consistency:
- Line 46:
"stopSearch": "Stop searching"→ should be Latvian (e.g., "Apturēt meklēšanu")- Line 262:
"currentPassword": "Your Current Password"→ should be Latvian (e.g., "Jūsu pašreizējā parole")Also applies to: 262-262
frontend/src/views/files/FileListing.vue-1155-1163 (1)
1155-1163:target.closest("item")will never match - remove redundant check.
target.closest("item")looks for an HTML element with tag name<item>, which doesn't exist in the DOM (the Vue<item>component renders as a<div class="item">). This check is redundant since line 1159 already checkstarget.closest(".item").🔧 Suggested fix
const handleEmptyAreaClick = (e: MouseEvent) => { const target = e.target; if (!(target instanceof HTMLElement)) return; - if (target.closest("item") || target.closest(".item")) return; + if (target.closest(".item")) return; if (target.closest(".context-menu")) return; fileStore.selected = []; };cmd/users_update.go-54-65 (1)
54-65: MissingQuotaFilefield in defaults initialization.The
defaultsstruct is missing theQuotaFilefield which exists in bothsettings.UserDefaultsandusers.User. This means if a user has a customQuotaFileset, it won't be preserved through the update flow viagetUserDefaults.Proposed fix
defaults := settings.UserDefaults{ Scope: user.Scope, TmpDir: user.TmpDir, TrashDir: user.TrashDir, + QuotaFile: user.QuotaFile, Locale: user.Locale, ViewMode: user.ViewMode, SingleClick: user.SingleClick, RedirectAfterCopyMove: user.RedirectAfterCopyMove, Perm: user.Perm, Sorting: user.Sorting, Commands: user.Commands, }frontend/src/i18n/es_CO.json-50-55 (1)
50-55: Missing Spanish translations for new keys.Several newly added keys are left in English instead of being translated to Spanish. This creates an inconsistent user experience for Spanish-speaking users.
Untranslated keys include:
buttons.stopSearch,buttons.editAsText,buttons.increaseFontSize,buttons.decreaseFontSize- All CSV-related strings under
fileslogin.passwordTooShort,login.logout_reasons.inactivitysettings.aceEditorTheme,settings.hideLoginButton,settings.currentPasswordWould you like me to open an issue to track the missing Spanish translations?
Also applies to: 95-103, 130-133, 200-200, 208-208, 287-287
cmd/utils.go-121-129 (1)
121-129: Config parse error check uses incorrect type comparison.The
errors.Is(err, viper.ConfigParseError{})check will not work as intended becauseerrors.Iscompares error values, not types. A zero-valueConfigParseError{}won't match actual parse errors.Suggested fix
// Read in configuration if err := v.ReadInConfig(); err != nil { - if errors.Is(err, viper.ConfigParseError{}) { + var parseErr viper.ConfigParseError + if errors.As(err, &parseErr) { return nil, err } log.Println("No config file used")frontend/src/i18n/zh_CN.json-50-55 (1)
50-55: Incomplete translation:stopSearchremains in English.The key
stopSearchhas the value "Stop searching" which should be translated to Chinese for consistency with the rest of the file. Consider: "停止搜索"frontend/src/i18n/zh_CN.json-296-297 (1)
296-297: Incomplete translation:currentPasswordremains in English.The key
currentPasswordhas the value "Your Current Password" which should be translated to Chinese. Consider: "您当前的密码"frontend/src/i18n/pt_PT.json-87-96 (1)
87-96: CSV-related strings untranslated.All new CSV-related strings (
noPreview,csvTooLarge,csvLoadFailed,showingRows,columnSeparator,csvSeparators.*) remain in English. Consider translating these to Portuguese.frontend/src/i18n/pt_PT.json-122-126 (1)
122-126: Login-related strings untranslated.The new keys
passwordTooShortandlogout_reasons.inactivityare in English. These user-facing messages should be translated to Portuguese.frontend/src/i18n/pt_PT.json-193-193 (1)
193-193: Settings strings partially untranslated.
aceEditorTheme,hideLoginButton, andcurrentPasswordremain in English while nearby additions likebrandingDirectoryPathandbrandingHelpare translated. Consider completing the Portuguese translations for consistency.Also applies to: 202-202, 279-280
frontend/src/i18n/pt_PT.json-49-54 (1)
49-54: Multiple untranslated strings in buttons section.The new button keys (
discardChanges,stopSearch,saveChanges,editAsText,increaseFontSize,decreaseFontSize) are all in English. These should be translated to Portuguese for consistency with the existing translations in this file.cmd/root.go-318-324 (1)
318-324: Variable shadowing:vshadows the function parameter.On line 321, the variable
vin theelse if v := os.Getenv("FB_BASEURL")shadows the function parameterv *viper.Viper. While this works correctly because the Viper parameter isn't used within this block, it reduces code clarity and could lead to bugs during future modifications.Suggested fix
if v.IsSet("baseURL") { server.BaseURL = v.GetString("baseURL") // TODO(remove): remove after July 2026. - } else if v := os.Getenv("FB_BASEURL"); v != "" { + } else if baseURLEnv := os.Getenv("FB_BASEURL"); baseURLEnv != "" { log.Println("DEPRECATION NOTICE: Environment variable FB_BASEURL has been deprecated, use FB_BASE_URL instead") - server.BaseURL = v + server.BaseURL = baseURLEnv }
| user := func(u *users.User) error { | ||
| u.Rules = append(u.Rules[:i], u.Rules[f+1:]...) | ||
| return d.store.Users.Save(u) | ||
| return st.Users.Save(u) | ||
| } | ||
|
|
||
| global := func(s *settings.Settings) error { | ||
| s.Rules = append(s.Rules[:i], s.Rules[f+1:]...) | ||
| return d.store.Settings.Save(s) | ||
| return st.Settings.Save(s) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's examine the full context of cmd/rule_rm.go around lines 56-64
head -80 cmd/rule_rm.go | tail -40Repository: hostinger/filebrowser
Length of output: 922
🏁 Script executed:
#!/bin/bash
# Let's see the entire file to understand the structure and where indices come from
wc -l cmd/rule_rm.goRepository: hostinger/filebrowser
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Get the full file content to trace the logic
cat -n cmd/rule_rm.goRepository: hostinger/filebrowser
Length of output: 2197
🏁 Script executed:
#!/bin/bash
# Search for runRules function definition
ast-grep --pattern 'func runRules($_) {
$$$
}'Repository: hostinger/filebrowser
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search for runRules function definition
rg "func runRules" --type goRepository: hostinger/filebrowser
Length of output: 214
🏁 Script executed:
#!/bin/bash
# Search where runRules is defined and how it uses the callbacks
rg "runRules" -A 20 --type goRepository: hostinger/filebrowser
Length of output: 1157
🏁 Script executed:
#!/bin/bash
# Get the complete runRules function from cmd/rules.go
rg "func runRules" -A 40 --type go cmd/rules.goRepository: hostinger/filebrowser
Length of output: 808
🏁 Script executed:
#!/bin/bash
# Let's also check the entire rules.go file to understand the context better
cat -n cmd/rules.go | head -100Repository: hostinger/filebrowser
Length of output: 2851
🏁 Script executed:
#!/bin/bash
# Verify that strconv.Atoi can indeed parse negative numbers
python3 << 'EOF'
import subprocess
result = subprocess.run(['go', 'run', '-'], input=b'''package main
import (
"fmt"
"strconv"
)
func main() {
i, _ := strconv.Atoi("-5")
fmt.Println("Parsed:", i)
}
''', capture_output=True, cwd='/tmp')
print(result.stdout.decode() if result.stdout else result.stderr.decode())
EOFRepository: hostinger/filebrowser
Length of output: 189
🏁 Script executed:
#!/bin/bash
# Check if there's any validation of indices elsewhere before the closures are called
rg "len.*Rules" --type go cmd/ | head -20Repository: hostinger/filebrowser
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check rules_add.go to see if it does bounds validation for comparison
cat -n cmd/rules_add.goRepository: hostinger/filebrowser
Length of output: 1888
Add bounds validation before slice operations to prevent panics.
The slice operations u.Rules[:i] and u.Rules[f+1:] will panic if indices exceed slice bounds or are negative. Since i and f are parsed directly from user input via strconv.Atoi() with no validation, callers can trigger panics by passing invalid indices like -1 or values exceeding the Rules length. Add defensive bounds checks in the user and global closures before performing the slice operations.
🤖 Prompt for AI Agents
In `@cmd/rule_rm.go` around lines 56 - 64, The closures user and global perform
unsafe slice removals using u.Rules[:i] and u.Rules[f+1:] without validating i
and f; add defensive bounds checks at the start of each closure to ensure i and
f are within 0 <= i <= f < len(u.Rules) (and similarly for len(s.Rules)) and
return a descriptive error instead of proceeding if the indices are invalid;
keep the calls to st.Users.Save(u) and st.Settings.Save(s) unchanged but only
call them after validation.
| if (files.length === 1) { | ||
| url += encodeURIComponent(files[0]) + "?"; | ||
| url += files[0] + "?"; | ||
| } else { |
There was a problem hiding this comment.
Missing URL encoding for single-file path.
The single-file case appends files[0] directly to the URL path without encoding. If the filename contains special characters (spaces, #, ?, &, unicode, etc.), the resulting URL will be malformed or incorrect.
In contrast, the multi-file case (lines 48-54) correctly encodes the arg string before appending it as a query parameter.
🐛 Proposed fix
if (files.length === 1) {
- url += files[0] + "?";
+ url += encodeURIComponent(files[0]) + "?";
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (files.length === 1) { | |
| url += encodeURIComponent(files[0]) + "?"; | |
| url += files[0] + "?"; | |
| } else { | |
| if (files.length === 1) { | |
| url += encodeURIComponent(files[0]) + "?"; | |
| } else { |
🤖 Prompt for AI Agents
In `@frontend/src/api/pub.ts` around lines 43 - 45, The single-file branch appends
files[0] raw to the URL, which breaks for spaces/special chars; change it to
append an encoded path segment (e.g., use encodeURIComponent(files[0])) instead
of files[0] so the URL is properly escaped (keep the trailing "?" as before) —
mirror the encoding approach used in the multi-file branch that encodes arg.
| const item = JSON.parse(line) as ResourceItem; | ||
| item.url = `/files${base}` + url.encodePath(item.path); | ||
| if (item.isDir) { | ||
| item.url += "/"; | ||
| } |
There was a problem hiding this comment.
Property name mismatch: backend sends dir, but code checks isDir.
Looking at the backend in http/search.go (line 65), the response sends "dir": f.IsDir(). However, this code checks item.isDir. This will cause the trailing slash to never be appended to directory URLs.
Proposed fix
const item = JSON.parse(line) as ResourceItem;
item.url = `/files${base}` + url.encodePath(item.path);
- if (item.isDir) {
+ if (item.dir) {
item.url += "/";
}
+ item.isDir = item.dir;
callback(item);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const item = JSON.parse(line) as ResourceItem; | |
| item.url = `/files${base}` + url.encodePath(item.path); | |
| if (item.isDir) { | |
| item.url += "/"; | |
| } | |
| const item = JSON.parse(line) as ResourceItem; | |
| item.url = `/files${base}` + url.encodePath(item.path); | |
| if (item.dir) { | |
| item.url += "/"; | |
| } | |
| item.isDir = item.dir; |
🤖 Prompt for AI Agents
In `@frontend/src/api/search.ts` around lines 41 - 45, The code parses each line
into a ResourceItem but checks item.isDir while the backend sends "dir", so
directory URLs never get a trailing slash; update the parsing/logic in
frontend/src/api/search.ts to use the backend property (use item.dir) or
explicitly map it (e.g., set item.isDir = item.dir after JSON.parse) and then
use that mapped value when appending the "/" to item.url; ensure the
ResourceItem type/signature is updated accordingly so future checks refer to the
correct property name (item.dir or item.isDir consistently).
| for (const line of lines) { | ||
| if (line) { | ||
| const item = JSON.parse(line) as ResourceItem; | ||
| item.url = `/files${base}` + url.encodePath(item.path); | ||
| if (item.isDir) { | ||
| item.url += "/"; | ||
| } | ||
| callback(item); | ||
| } | ||
| } |
There was a problem hiding this comment.
Same property name mismatch in fallback path.
The fallback path has the same issue - checking item.isDir when the backend sends dir.
Proposed fix
const lines = text.split(/\n/);
for (const line of lines) {
if (line) {
const item = JSON.parse(line) as ResourceItem;
item.url = `/files${base}` + url.encodePath(item.path);
- if (item.isDir) {
+ if (item.dir) {
item.url += "/";
}
+ item.isDir = item.dir;
callback(item);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const line of lines) { | |
| if (line) { | |
| const item = JSON.parse(line) as ResourceItem; | |
| item.url = `/files${base}` + url.encodePath(item.path); | |
| if (item.isDir) { | |
| item.url += "/"; | |
| } | |
| callback(item); | |
| } | |
| } | |
| for (const line of lines) { | |
| if (line) { | |
| const item = JSON.parse(line) as ResourceItem; | |
| item.url = `/files${base}` + url.encodePath(item.path); | |
| if (item.dir) { | |
| item.url += "/"; | |
| } | |
| item.isDir = item.dir; | |
| callback(item); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@frontend/src/api/search.ts` around lines 55 - 64, The loop in
frontend/src/api/search.ts parses each line into item and then checks
item.isDir, but the backend sends the boolean as item.dir; update the fallback
handling so it uses the correct property (e.g., replace the check if
(item.isDir) with if (item.dir) or normalize the object by setting item.isDir =
item.dir right after parsing) before appending "/" and invoking callback(item).
| let lastThrottleTime = 0; | ||
|
|
||
| const throttledCalculateSpeed = (sentBytes: number, oldSentBytes: number) => { | ||
| const now = Date.now(); | ||
| if (now - lastThrottleTime < 100) { | ||
| return; | ||
| } | ||
|
|
||
| lastThrottleTime = now; | ||
| calculateSpeed(sentBytes, oldSentBytes); | ||
| }; |
There was a problem hiding this comment.
Speed calculation is inaccurate when throttling skips updates.
The throttled wrapper skips calculateSpeed calls, but calculateSpeed uses oldSentBytes from the watch callback (value from previous watch trigger) while elapsedTime is computed from lastSpeedUpdate (timestamp from last calculation). When updates are throttled, these values become mismatched:
elapsedTime= time since last calculationbytesSinceLastUpdate= bytes since last watch trigger
This causes under-reported speeds when throttling is active.
🔧 Proposed fix: Track lastSentBytes separately
let lastSpeedUpdate: number = 0;
let recentSpeeds: number[] = [];
+let lastSentBytesAtCalculation: number = 0;
let lastThrottleTime = 0;
-const throttledCalculateSpeed = (sentBytes: number, oldSentBytes: number) => {
+const throttledCalculateSpeed = (sentBytes: number) => {
const now = Date.now();
if (now - lastThrottleTime < 100) {
return;
}
lastThrottleTime = now;
- calculateSpeed(sentBytes, oldSentBytes);
+ calculateSpeed(sentBytes);
};
-const calculateSpeed = (sentBytes: number, oldSentBytes: number) => {
+const calculateSpeed = (sentBytes: number) => {
// Reset the state when the uploads batch is complete
if (sentBytes === 0) {
lastSpeedUpdate = 0;
recentSpeeds = [];
+ lastSentBytesAtCalculation = 0;
eta.value = Infinity;
speed.value = 0;
return;
}
const elapsedTime = (Date.now() - (lastSpeedUpdate ?? 0)) / 1000;
- const bytesSinceLastUpdate = sentBytes - oldSentBytes;
+ const bytesSinceLastUpdate = sentBytes - lastSentBytesAtCalculation;
const currentSpeed = bytesSinceLastUpdate / elapsedTime;
recentSpeeds.push(currentSpeed);
if (recentSpeeds.length > 5) {
recentSpeeds.shift();
}
const recentSpeedsAverage =
recentSpeeds.reduce((acc, curr) => acc + curr) / recentSpeeds.length;
// Use the current speed for the first update to avoid smoothing lag
if (recentSpeeds.length === 1) {
speed.value = currentSpeed;
}
speed.value = recentSpeedsAverage * 0.2 + speed.value * 0.8;
lastSpeedUpdate = Date.now();
+ lastSentBytesAtCalculation = sentBytes;
calculateEta();
};🤖 Prompt for AI Agents
In `@frontend/src/components/prompts/UploadFiles.vue` around lines 114 - 124, The
throttling wrapper causes mismatched deltas because calculateSpeed uses
oldSentBytes from the watch while elapsedTime is measured from lastSpeedUpdate;
fix by introducing and maintaining a separate lastSentBytes variable that is
updated only when calculateSpeed actually runs: add a module-level
lastSentBytes, use it inside calculateSpeed to compute bytesSinceLastCalc =
sentBytes - lastSentBytes, update lastSentBytes and lastSpeedUpdate at the end
of calculateSpeed, and have throttledCalculateSpeed call
calculateSpeed(sentBytes) (or pass current sentBytes) instead of relying on
oldSentBytes so throttled calls compute correct speed.
| saveAction: async () => { | ||
| await save(); | ||
| next(); | ||
| }, |
There was a problem hiding this comment.
Navigation proceeds even if save fails.
In the saveAction callback, save() is called without throwError, so if the save operation fails, next() will still be executed, navigating away despite the failed save. This differs from the close() logic (lines 226-230) which properly uses save(true) and catches errors to prevent navigation.
Proposed fix
saveAction: async () => {
- await save();
- next();
+ try {
+ await save(true);
+ next();
+ } catch {}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| saveAction: async () => { | |
| await save(); | |
| next(); | |
| }, | |
| saveAction: async () => { | |
| try { | |
| await save(true); | |
| next(); | |
| } catch {} | |
| }, |
🤖 Prompt for AI Agents
In `@frontend/src/views/files/Editor.vue` around lines 156 - 159, The saveAction
currently calls save() then always calls next(), so navigation proceeds even if
the save fails; change saveAction to await save(true) (to throw on failure) and
only call next() when that await succeeds, i.e. wrap the await save(true) in a
try/catch or let the thrown error prevent calling next(); reference saveAction,
save(), and next() when updating the callback so failed saves block navigation
just like close() does.
| } | ||
|
|
||
| cmd := exec.Command(command[0], command[1:]...) //nolint:gosec | ||
| cmd := exec.Command(command[0], command[1:]...) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "commands.go"Repository: hostinger/filebrowser
Length of output: 100
🏁 Script executed:
git ls-files | grep -E "(commands|runner)" | head -20Repository: hostinger/filebrowser
Length of output: 187
🏁 Script executed:
cat -n http/commands.go | head -100Repository: hostinger/filebrowser
Length of output: 2960
🏁 Script executed:
cat -n runner/parser.goRepository: hostinger/filebrowser
Length of output: 871
🏁 Script executed:
cat -n runner/commands.go | head -150Repository: hostinger/filebrowser
Length of output: 4428
Remove nolint:gosec exemption or add input sanitization when Shell mode is enabled.
The safeguards mentioned are incomplete. While ParseCommand correctly parses the command name (validated against whitelist), it does not sanitize input. When Shell is configured in settings, the entire unparsed raw string is passed directly to the shell (runner/parser.go, lines 20–21), bypassing the whitelist protection. A user with an allowed command can inject shell metacharacters to execute arbitrary commands.
For example, a user with whoami whitelisted could input whoami; rm -rf / and have both commands executed when Shell mode is enabled.
Mitigation options:
- Restore
nolint:goseconly ifShellis disabled or definitively empty in your deployment - Implement input sanitization to escape shell metacharacters when Shell mode is active
- Enforce no-shell mode by disallowing
Shellconfiguration
| timeout := time.NewTimer(searchPingInterval * time.Second) | ||
| defer timeout.Stop() | ||
| for { | ||
| var err error | ||
| var infoBytes []byte | ||
| select { | ||
| case info := <-response: | ||
| if info == nil { | ||
| return | ||
| } | ||
| infoBytes, err = json.Marshal(info) | ||
| case <-timeout.C: | ||
| // Send a heartbeat packet | ||
| infoBytes = nil | ||
| case <-ctx.Done(): | ||
| return | ||
| } | ||
| if err != nil { | ||
| cancel(err) | ||
| return | ||
| } | ||
| _, err = w.Write(infoBytes) | ||
| if err == nil { | ||
| _, err = w.Write([]byte("\n")) | ||
| } | ||
| if err != nil { | ||
| cancel(err) | ||
| return | ||
| } | ||
| if flusher, ok := w.(http.Flusher); ok { | ||
| flusher.Flush() | ||
| } | ||
| } |
There was a problem hiding this comment.
Timer not reset after heartbeat, causing only one heartbeat to be sent.
The timer is created once but never reset after firing. After the first heartbeat at 5 seconds, timeout.C will never fire again because time.Timer channels only receive one value unless explicitly reset with Reset().
Proposed fix
go func() {
defer wg.Done()
// Avoid connection timeout
timeout := time.NewTimer(searchPingInterval * time.Second)
defer timeout.Stop()
for {
var err error
var infoBytes []byte
select {
case info := <-response:
if info == nil {
return
}
infoBytes, err = json.Marshal(info)
+ if !timeout.Stop() {
+ select {
+ case <-timeout.C:
+ default:
+ }
+ }
+ timeout.Reset(searchPingInterval * time.Second)
case <-timeout.C:
// Send a heartbeat packet
infoBytes = nil
+ timeout.Reset(searchPingInterval * time.Second)
case <-ctx.Done():
return
}🤖 Prompt for AI Agents
In `@http/search.go` around lines 25 - 57, The timer is created once (timeout :=
time.NewTimer(...)) but never reset so timeout.C only fires once; fix by
resetting or replacing the timer: either swap to a time.NewTicker and use
ticker.C (stopping it on return), or, if keeping the timer, call
timeout.Reset(searchPingInterval * time.Second) inside the timeout.C case (right
after setting infoBytes = nil) so the heartbeat continues each interval; keep
the existing defer timeout.Stop() (or stop the ticker) and leave the rest of the
loop logic (response, ctx, cancel, w, flusher) unchanged.
| > ```docker | ||
| > FROM filebrowser/filebrowser | ||
| > RUN sudo apt install p7zip-full | ||
| > ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
filebrowser/filebrowser docker image base OS alpine debian
💡 Result:
The official filebrowser/filebrowser Docker image is Alpine-based (there’s a plain Alpine image plus an s6-overlay variant). It is not Debian-based by default. [1][2][3]
Sources:
- File Browser installation docs (Alpine + s6-overlay images). [1]
- Official filebrowser/filebrowser Docker Hub listing (image tags including s6 variant). [2]
- Example image scan / metadata showing Alpine base. [3]
🏁 Script executed:
#!/bin/bash
# Search for filebrowser Dockerfile in common locations
fd -e Dockerfile | head -20
# Also try to find any info about filebrowser in repo
rg -i "filebrowser" -t md | head -20Repository: hostinger/filebrowser
Length of output: 3317
🏁 Script executed:
#!/bin/bash
# Check if there's information about the image in the current repo
git ls-files | grep -i dockerRepository: hostinger/filebrowser
Length of output: 315
🏁 Script executed:
#!/bin/bash
# Read the main Dockerfile to check base image and structure
head -30 DockerfileRepository: hostinger/filebrowser
Length of output: 1234
🏁 Script executed:
#!/bin/bash
# Check the Dockerfile.s6 as well
head -30 Dockerfile.s6Repository: hostinger/filebrowser
Length of output: 729
🏁 Script executed:
#!/bin/bash
# Also check the www/Dockerfile since it might be related to documentation
cat www/DockerfileRepository: hostinger/filebrowser
Length of output: 187
🏁 Script executed:
#!/bin/bash
# Read the full command-execution.md to understand the context
cat -n www/docs/command-execution.mdRepository: hostinger/filebrowser
Length of output: 2981
Docker example is fundamentally broken and cannot be fixed with suggested apk command.
The filebrowser/filebrowser base image uses BusyBox (minimal, no package managers). The suggested apk add --no-cache p7zip won't work either since BusyBox doesn't include apk.
To extend filebrowser with additional tools like 7z, build from a full OS image instead:
-FROM filebrowser/filebrowser
-RUN sudo apt install p7zip-full
+FROM alpine:latest
+RUN apk add --no-cache p7zip
+COPY --from=filebrowser/filebrowser:latest / /Alternatively, use the s6-overlay variant as a base (e.g., FROM filebrowser/filebrowser:s6), which includes Alpine and package managers.
🤖 Prompt for AI Agents
In `@www/docs/command-execution.md` around lines 51 - 54, The Docker example is
invalid because the base image "FROM filebrowser/filebrowser" is BusyBox-based
and lacks package managers; replace that line by either building FROM a full
distro (e.g., Ubuntu/Debian) and install p7zip-full with apt and then install or
copy the filebrowser binary, or switch to the s6 variant "FROM
filebrowser/filebrowser:s6" (Alpine-based) and install p7zip via apk; update the
document to show one of these two valid approaches and remove the broken "RUN
sudo apt install p7zip-full" example.
This reverts commit 4f5c0f5.
20 participants
Summary by CodeRabbit
Release Notes
New Features
Improvements
Updates
✏️ Tip: You can customize this high-level summary in your review settings.