feat: merge upstream#78
Conversation
📝 WalkthroughWalkthroughThis PR introduces broad changes across backend, frontend, CI/CD, and docs. Key themes: new settings (minimumPasswordLength, file/dir modes), extensive CLI error-propagation refactor, password validation/common-password checks, TUS upload handlers with active-upload tracking, command execution authorization changes, file operation mode propagation, frontend upload/store/API rewrites, cancellable requests and preselection, documentation site addition, and multiple tooling/configuration updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant M as main
participant C as cmd.Execute
participant RC as rootCmd (Cobra)
participant E as errors.GetExitCode
U->>M: start program
M->>C: Execute()
C->>RC: RunE(...)
RC-->>C: error or nil
alt error != nil
C-->>M: error
M->>E: GetExitCode(error)
E-->>M: exit code (int)
M-->>U: os.Exit(code)
else success
C-->>M: nil
M-->>U: normal exit
end
sequenceDiagram
autonumber
actor CL as Client
participant S as Server (/api/tus)
participant AC as Active Uploads (ttlcache)
participant FS as Filesystem
CL->>S: POST /api/tus (Upload-Length, path, overwrite?)
S->>FS: mkdir (DirMode), open file (FileMode)
S->>S: getUploadLength()
S->>AC: registerUpload(id, length)
S-->>CL: 201 Created (Location: /api/tus/{id})
loop upload chunks
CL->>S: PATCH /api/tus/{id} (Content-Type: application/offset+octet-stream, Upload-Offset)
S->>AC: keepUploadActive(id)
S->>FS: append chunk
alt offset >= length
S->>AC: completeUpload(id)
S-->>CL: 204 No Content
else
S-->>CL: 204 No Content (Upload-Offset)
end
end
CL->>S: HEAD /api/tus/{id}
S->>AC: getActiveUploadLength(id)
S-->>CL: 200 OK (Upload-Offset, Upload-Length)
opt cancel
CL->>S: DELETE /api/tus/{id}
S->>AC: completeUpload(id)
S->>FS: remove file
S-->>CL: 204 No Content
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 81
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (34)
http/share.go (2)
94-101: Increase share link entropy and use RawURLEncoding (no padding) for URL safety48-bit IDs are collision-prone at scale; bump to 96–128 bits. RawURLEncoding avoids '=' padding in paths.
- bytes := make([]byte, 6) + bytes := make([]byte, 12) // 96 bits; consider 16 for 128 bits _, err := rand.Read(bytes) if err != nil { return http.StatusInternalServerError, err } - str := base64.URLEncoding.EncodeToString(bytes) + str := base64.RawURLEncoding.EncodeToString(bytes)
131-138: Token size is excessive; 256-bit token is ample and shorter; also use RawURLEncodingReduce storage and header size while keeping strong security.
var token string if len(hash) > 0 { - tokenBuffer := make([]byte, 96) + tokenBuffer := make([]byte, 32) // 256 bits if _, err := rand.Read(tokenBuffer); err != nil { return http.StatusInternalServerError, err } - token = base64.URLEncoding.EncodeToString(tokenBuffer) + token = base64.RawURLEncoding.EncodeToString(tokenBuffer) }docker/s6/custom-cont-init.d/20-config (2)
3-7: First-run robustness: ensure /config exists before copying; set restrictive permscp will fail if /config is missing; also consider tighter perms for settings.json.
-# Ensure configuration exists -if [ ! -f "/config/settings.json" ]; then - cp -a /defaults/settings.json /config/settings.json -fi +# Ensure configuration exists +mkdir -p /config +if [ ! -f "/config/settings.json" ]; then + cp -a /defaults/settings.json /config/settings.json + chmod 0640 /config/settings.json +fi
8-13: Avoid chown failures on missing dirsCreate target dirs before chown to reduce noisy errors and ensure ownership is correct.
-# permissions -chown abc:abc \ - /config/settings.json \ - /database \ - /srv +# permissions +mkdir -p /database /srv +chown abc:abc \ + /config/settings.json \ + /database \ + /srvhostinger/archive.go (2)
54-66: Prevent Zip Slip (path traversal) when extractingNameInArchive can escape dst (e.g., ../../etc/passwd). Enforce that the final path stays within dst.
- fullpath := filepath.Join(dst, filepath.Clean(file.NameInArchive)) + fullpath := filepath.Join(dst, filepath.Clean(file.NameInArchive)) + // ensure extraction stays within destination + rel, err := filepath.Rel(filepath.Clean(dst), fullpath) + if err != nil || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || rel == ".." { + return fmt.Errorf("extract path escapes destination: %q", file.NameInArchive) + }
41-47: Close archive reader to avoid FD leakreader is never closed; add a deferred Close.
- reader, err := afs.Open(src) + reader, err := afs.Open(src) if err != nil { return fmt.Errorf("archive open: %w", err) } + defer reader.Close()frontend/src/components/files/VideoPlayer.vue (1)
61-67: Normalize lang (case/underscore) to hit available packs; keep fallback.Without normalization, values like en-US or pt-BR miss packs and fall back to English.
Apply:
- const lang = document.documentElement.lang; - const languagePack = await ( - languageImports[lang] || languageImports.en - )?.(); - const code = languageImports[lang] ? lang : "en"; - videojs.addLanguage(code, languagePack.default); + const langRaw = document.documentElement.lang || "en"; + const lang = langRaw.toLowerCase().replace("_", "-"); + const key = languageImports[lang] ? lang : "en"; + const languagePack = await (languageImports[key] || languageImports.en)?.(); + videojs.addLanguage(key, languagePack.default);- const langOpt = { language: code }; + const langOpt = { language: key };Also applies to: 74-74
http/data.go (1)
56-60: Do not crash the server on settings retrieval failure.log.Fatalf inside a request handler will terminate the process. Return 500 instead.
Apply:
- if err != nil { - log.Fatalf("ERROR: couldn't get settings: %v\n", err) - return - } + if err != nil { + log.Printf("ERROR: couldn't get settings: %v\n", err) + http.Error(w, "500 Internal Server Error", http.StatusInternalServerError) + return + }frontend/src/api/utils.ts (1)
27-36: Respect the auth flag when sending the JWT header.X-Auth is currently sent unconditionally, even when auth=false is passed (e.g., public endpoints). This leaks the token unnecessarily.
- res = await fetch(`${baseURL}${url}`, { - headers: { - "X-Auth": authStore.jwt, - ...headers, - }, - ...rest, - }); + res = await fetch(`${baseURL}${url}`, { + headers: { + ...(auth ? { "X-Auth": authStore.jwt } : {}), + ...headers, + }, + ...rest, + });frontend/src/views/settings/Global.vue (2)
311-317: Duplicate/contradictory assignment to newSettings.shell.You compute shell from settings.value.shell and then immediately overwrite it from shellValue. Remove the first block to avoid confusion.
- shell: - settings.value?.shell - .join(" ") - .trim() - .split(" ") - .filter((s: string) => s !== "") ?? [], commands: {},Also applies to: 336-339
355-375: parseBytes crashes when unit is omitted (matches[3] can be undefined).Guard before calling toUpperCase and handle plain numbers as bytes.
- const matches = input.match(regex); - if (matches) { - const size = parseFloat(matches[1].concat(matches[2] || "")); - let unit: keyof SettingsUnit = - matches[3].toUpperCase() as keyof SettingsUnit; - if (!unit.endsWith("B")) { - unit += "B"; - } + const matches = input.match(regex); + if (matches) { + const size = parseFloat((matches[1] || "") + (matches[2] || "")); + let unitRaw = (matches[3] || "B").toUpperCase(); + let unit = unitRaw.endsWith("B") ? unitRaw : unitRaw + "B"; const units: SettingsUnit = { KB: 1024, MB: 1024 ** 2, GB: 1024 ** 3, TB: 1024 ** 4, }; return size * (units[unit as keyof SettingsUnit] || 1); } else { return 1024 ** 2; }frontend/src/views/files/FileListing.vue (1)
630-649: Preselect join may miss a slash; normalize base before concatenation.removePrefix(route.path) might not end with “/”. Ensure a trailing slash so preselect matches later consumers.
- const preselect = removePrefix(route.path) + items[0].name; + const base = removePrefix(route.path.endsWith("/") ? route.path : route.path + "/"); + const preselect = base + items[0].name;fileutils/dir.go (2)
28-33: Handle open errors and close directory handles.Ignoring afs.Open errors can lead to nil deref on Readdir; also leaks the handle. Same issue below in CopyDirScoped.
- dir, _ := afs.Open(source) - obs, err := dir.Readdir(-1) + dir, err := afs.Open(source) + if err != nil { + return err + } + defer dir.Close() + obs, err := dir.Readdir(-1)
81-86: Same resource handling fix for scoped path.- dir, _ := afs.Open(source) - obs, err := dir.Readdir(-1) + dir, err := afs.Open(source) + if err != nil { + return err + } + defer dir.Close() + obs, err := dir.Readdir(-1)frontend/src/views/files/Preview.vue (1)
159-160: Avoid prefetching empty URLsEmpty href may trigger unexpected fetches. Render links conditionally.
-<link rel="prefetch" :href="previousRaw" /> -<link rel="prefetch" :href="nextRaw" /> +<link v-if="previousRaw" rel="prefetch" :href="previousRaw" /> +<link v-if="nextRaw" rel="prefetch" :href="nextRaw" />frontend/src/views/Layout.vue (1)
52-59: Route watcher may not fire reliably; watch a getter instead.In Vue Router 4, watch(() => route.fullPath) is the safer pattern.
-watch(route, () => { +watch(() => route.fullPath, () => { fileStore.selected = []; fileStore.multiple = false; contextMenuStore.hide(); if (layoutStore.currentPromptName !== "success") { layoutStore.closeHovers(); } });frontend/src/components/prompts/Share.vue (1)
251-255: Bug: wrong time ordering; expire treated as ms, but it’s seconds.new Date(expire) expects milliseconds. This mis-sorts links.
- this.links = this.links.sort((a, b) => { - if (a.expire === 0) return -1; - if (b.expire === 0) return 1; - return new Date(a.expire) - new Date(b.expire); - }); + this.links = this.links.sort((a, b) => { + if (a.expire === 0) return -1; + if (b.expire === 0) return 1; + // expire is UNIX seconds; compare numerically (ascending) + return a.expire - b.expire; + });cmd/rules_add.go (1)
35-37: Don’t panic on invalid regex; return an error instead.regexp.MustCompile will crash the CLI on user input. Compile and propagate the error.
- if regex { - regexp.MustCompile(exp) - } + if regex { + if _, err := regexp.Compile(exp); err != nil { + return err + } + }frontend/src/api/files.ts (1)
65-87: Downloads opened via window.open are unauthenticated (X-Auth header not sent)
- Problem: download(...) in frontend/src/api/files.ts builds a raw URL and calls window.open; window.open does not send the X-Auth header that fetchURL/postResources set, and createURL only builds a URL (does not add auth) — authenticated downloads will 401.
- Remediation (choose one):
- Fetch via fetchURL (sends X-Auth), stream to a Blob and use URL.createObjectURL / an click to trigger the download.
- Or change the backend to accept a query token or cookie-based session, then use createURL/getDownloadURL to open the URL.
Location: frontend/src/api/files.ts — function download(format: any, ...files: string[])
fileutils/copy.go (1)
63-75: Update usages after renaming and silence exhaustive switch lint.Also route calls through the renamed receiver and pass it to nested functions.
- info, err := fs.Stat(src) + info, err := afs.Stat(src) if err != nil { return err } - switch info.Mode() & os.ModeType { + //nolint:exhaustive // fs.FileMode is a bitmask; we only care about dir, symlink, and regular files here. + switch info.Mode() & os.ModeType { case os.ModeDir: - return CopyDirScoped(fs, src, dst, fileMode, dirMode, scope) + return CopyDirScoped(afs, src, dst, fileMode, dirMode, scope) case os.ModeSymlink: - return CopySymLinkScoped(fs, src, dst, scope) + return CopySymLinkScoped(afs, src, dst, scope) default: - return CopyFile(fs, src, dst, fileMode, dirMode) + return CopyFile(afs, src, dst, fileMode, dirMode) }cmd/users.go (1)
99-156: Handle dateFormat and hideDotfiles in getUserDefaults.Wire the new flags into defaults.
case "singleClick": defaults.SingleClick, err = getBool(flags, flag.Name) + case "dateFormat": + defaults.DateFormat, err = getBool(flags, flag.Name) + case "hideDotfiles": + defaults.HideDotfiles, err = getBool(flags, flag.Name)cmd/rule_rm.go (2)
35-41: Reject negative indices in argument validation.
strconv.Atoiaccepts “-1”, which would trigger a panic later. Enforce non-negative.- for _, arg := range args { - if _, err := strconv.Atoi(arg); err != nil { - return err - } - } + for _, arg := range args { + v, err := strconv.Atoi(arg) + if err != nil { + return err + } + if v < 0 { + return fmt.Errorf("index must be >= 0") + } + }
14-16: Remove or stop requiring the unused --index flag (CLI currently breaks).The command consumes positional args but requires a flag that isn’t read. Users running
rules rm 1will be forced to pass an unused--index.- rulesRmCommand.Flags().Uint("index", 0, "index of rule to remove") - _ = rulesRmCommand.MarkFlagRequired("index") + // Indices are provided as positional args: rm <index> [index_end]If you must keep the flag for compatibility, wire it up or drop
MarkFlagRequired.cmd/users_update.go (1)
55-65: Propagate QuotaFile through defaultsQuotaFile is part of UserDefaults but isn’t copied to/from defaults, so it cannot be updated via flags.
defaults := settings.UserDefaults{ Scope: user.Scope, TmpDir: user.TmpDir, TrashDir: user.TrashDir, + QuotaFile: user.QuotaFile, Locale: user.Locale, ViewMode: user.ViewMode, SingleClick: user.SingleClick, Perm: user.Perm, Sorting: user.Sorting, Commands: user.Commands, } ... user.Locale = defaults.Locale user.ViewMode = defaults.ViewMode user.SingleClick = defaults.SingleClick user.Perm = defaults.Perm user.Commands = defaults.Commands user.Sorting = defaults.Sorting +user.QuotaFile = defaults.QuotaFileAlso applies to: 70-79
http/resource.go (2)
39-47: Use configured DirMode when auto-creating Trash dir.Hard-coding 0775 ignores the new configurable modes and can cause inconsistent permissions.
Apply:
- err = d.user.Fs.MkdirAll(d.user.TrashDir, 0775) //nolint:gomnd + err = d.user.Fs.MkdirAll(d.user.TrashDir, d.settings.DirMode)
170-176: Use DirMode for Trash dir creation here as well.Same rationale as GET-path creation: honor configured permissions.
Apply:
- err = d.user.Fs.MkdirAll(dst, 0775) //nolint:gomnd + err = d.user.Fs.MkdirAll(dst, d.settings.DirMode)frontend/src/api/tus.ts (1)
10-15: Do not default ApiContent to a string; avoid returning boolean from an async upload.Defaulting to "" leaks a boolean into the Promise type and will confuse call sites. Require a valid Blob/File.
Apply:
-export async function upload( - filePath: string, - content: ApiContent = "", - overwrite = false, - onupload: any -) { +export async function upload( + filePath: string, + content: ApiContent, + overwrite = false, + onupload: any +) { @@ - // Exit early because of typescript, tus content can't be a string - if (content === "") { - return false; - } + if (!(content instanceof Blob)) { + throw new Error("Tus requires Blob/File content"); + }cmd/utils.go (1)
103-121: Replace panic/log.Fatal with proper error returns; fix user-facing typo.This wrapper should never exit the process. Return errors and let Cobra handle them. Also “Initialing” → “Initializing”.
- return func(cmd *cobra.Command, args []string) error { + return func(cmd *cobra.Command, args []string) error { data := &pythonData{hadDB: true} path := getStringParam(cmd.Flags(), "database") absPath, err := filepath.Abs(path) if err != nil { - panic(err) + return err } exists, err := dbExists(path) if err != nil { - panic(err) + return err } else if exists && cfg.noDB { - log.Fatal(absPath + " already exists") + return fmt.Errorf("%s already exists", absPath) } else if !exists && !cfg.noDB && !cfg.allowNoDB { - log.Fatal(absPath + " does not exist. Please run 'filebrowser config init' first.") + return fmt.Errorf("%s does not exist. Please run 'filebrowser config init' first.", absPath) } else if !exists && !cfg.noDB { - log.Println("Warning: filebrowser.db can't be found. Initialing in " + strings.TrimSuffix(absPath, "filebrowser.db")) + log.Println("Warning: filebrowser.db can't be found. Initializing in " + strings.TrimSuffix(absPath, "filebrowser.db")) } log.Println("Using database: " + absPath) data.hadDB = exists db, err := storm.Open(path, storm.BoltOptions(dbPerms, nil)) if err != nil { return err } defer db.Close() data.store, err = bolt.NewStorage(db) if err != nil { return err } return fn(cmd, args, data) }Also applies to: 123-135
fileutils/file.go (1)
70-79: Honour configured fileMode; don't overwrite with source mode.Chmod to source mode nullifies the passed-in fileMode. Either preserve source mode by contract (then drop fileMode param) or apply fileMode consistently. Suggest the latter here.
- info, err := afs.Stat(source) - if err != nil { - return err - } - err = afs.Chmod(dest, info.Mode()) + // Apply desired destination mode instead of inheriting source's. + err = afs.Chmod(dest, fileMode) if err != nil { return err }http/tus_handlers.go (1)
242-247: Prevent writing past declared Upload-Length; avoid O_APPEND.Limit the reader to remaining bytes and open without O_APPEND to respect the Seek.
- openFile, err := d.user.Fs.OpenFile(r.URL.Path, os.O_WRONLY|os.O_APPEND, d.settings.FileMode) + openFile, err := d.user.Fs.OpenFile(r.URL.Path, os.O_WRONLY, d.settings.FileMode) if err != nil { return http.StatusInternalServerError, fmt.Errorf("could not open file: %w", err) } defer openFile.Close() _, err = openFile.Seek(uploadOffset, 0) if err != nil { return http.StatusInternalServerError, fmt.Errorf("could not seek file: %w", err) } defer r.Body.Close() - bytesWritten, err := io.Copy(openFile, r.Body) + remaining := uploadLength - uploadOffset + if remaining < 0 { + return http.StatusConflict, fmt.Errorf("upload offset beyond length") + } + lr := io.LimitedReader{R: r.Body, N: remaining} + bytesWritten, err := io.Copy(openFile, &lr) if err != nil { return http.StatusInternalServerError, fmt.Errorf("could not write to file: %w", err) } newOffset := uploadOffset + bytesWritten w.Header().Set("Upload-Offset", strconv.FormatInt(newOffset, 10)) - if newOffset >= uploadLength { + if newOffset == uploadLength { completeUpload(file.RealPath()) _ = d.RunHook(func() error { return nil }, "upload", r.URL.Path, "", d.user) + } else if newOffset > uploadLength { + return http.StatusConflict, fmt.Errorf("received more data than declared Upload-Length") }Also applies to: 248-258, 259-266
cmd/root.go (2)
163-165: Use net.JoinHostPort for IPv6-safe address formatting.Concatenation breaks for IPv6 literals. JoinHostPort handles IPv6/zone IDs correctly.
- adr := server.Address + ":" + server.Port + adr := net.JoinHostPort(server.Address, server.Port)
508-510: Don’t log.Fatal in functions that return an error; return an error instead.This short-circuits RunE error handling and exit-code mapping.
- if username == "" || password == "" { - log.Fatal("username and password cannot be empty during quick setup") - } + if username == "" || password == "" { + return fmt.Errorf("username and password cannot be empty during quick setup") + }frontend/src/i18n/es_CO.json (2)
171-174: English strings in ES locale (search/settings/themes).- "images": "Images", + "images": "Imágenes", @@ - "minimumPasswordLength": "Minimum password length", + "minimumPasswordLength": "Longitud mínima de la contraseña", - "tusUploads": "Chunked Uploads", + "tusUploads": "Cargas en bloques", - "tusUploadsHelp": "File Browser supports chunked file uploads, allowing for the creation of efficient, reliable, resumable and chunked file uploads even on unreliable networks.", + "tusUploadsHelp": "File Browser admite cargas de archivos en bloques, permitiendo cargas eficientes, fiables y reanudables incluso en redes inestables.", - "tusUploadsChunkSize": "Indicates to maximum size of a request (direct uploads will be used for smaller uploads). You may input a plain integer denoting byte size input or a string like 10MB, 1GB etc.", + "tusUploadsChunkSize": "Indica el tamaño máximo por solicitud (se usarán cargas directas para archivos pequeños). Puede introducir un entero (bytes) o una cadena como 10MB, 1GB, etc.", - "tusUploadsRetryCount": "Number of retries to perform if a chunk fails to upload.", + "tusUploadsRetryCount": "Número de reintentos si un bloque falla.", @@ - "default": "System default", + "default": "Predeterminado del sistema", - "title": "Tema" + "title": "Tema"Also applies to: 198-206, 252-256
1-297: i18n audit — frontend/src/i18n/es_CO.json: 10 missing keys, 22 strings still in English
Missing keys (10) — add Spanish translations: prompts.skipTrashMessage, prompts.unarchive, prompts.unarchiveDestinationLocationMessage, prompts.unarchiveFolderNameMessage, prompts.unarchiveMessage, prompts.unarchiveOverwriteExisting, prompts.unsavedChanges, prompts.uploadFile, prompts.uploadFolder, prompts.write
Untranslated (22) — replace English values:
- buttons.copyDownloadLinkToClipboard = Copy download link to clipboard
- buttons.discardChanges = Discard
- buttons.edit = Edit
- buttons.fullScreen = Toggle full screen
- buttons.info = Info
- buttons.ok = OK
- buttons.preview = Preview
- prompts.deleteUser = Are you sure you want to delete this user?
- prompts.discardEditorChanges = Are you sure you wish to discard the changes you've made?
- prompts.resolution = Resolution
- search.images = Images
- search.pdf = PDF
- settings.admin = Admin
- settings.commandRunnerHelp = Here you can set commands that are executed in the named events. You must write one per line. The environment variables {0} and {1} will be available, being {0} relative to {1}. For more information about this feature and the available environment variables, please read the {2}.
- settings.disableUsedDiskPercentage = Disable used disk percentage graph
- settings.minimumPasswordLength = Minimum password length
- settings.themes.default = System default
- settings.tusUploads = Chunked Uploads
- settings.tusUploadsChunkSize = Indicates to maximum size of a request (direct uploads will be used for smaller uploads). You may input a plain integer denoting byte size input or a string like 10MB, 1GB etc.
- settings.tusUploadsHelp = File Browser supports chunked file uploads, allowing for the creation of efficient, reliable, resumable and chunked file uploads even on unreliable networks.
- settings.tusUploadsRetryCount = Number of retries to perform if a chunk fails to upload.
- upload.abortUpload = Are you sure you wish to abort?
Fix the missing keys and translate the English leftovers in frontend/src/i18n/es_CO.json.
| render: Text | ||
| description: | | ||
| Enter the version of FileBrowser you are using. | ||
| validations: |
There was a problem hiding this comment.
Remove or fix invalid "render: Text" on textarea.
GitHub Issue Forms’ textarea render expects a known language (e.g., text, shell, yaml) or can be omitted. "Text" (capital T) is not recognized.
Apply:
- render: Text
+ # render: text📝 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.
| render: Text | |
| description: | | |
| Enter the version of FileBrowser you are using. | |
| validations: | |
| # render: text | |
| description: | | |
| Enter the version of FileBrowser you are using. | |
| validations: |
🤖 Prompt for AI Agents
.github/ISSUE_TEMPLATE/bug_report.yml lines 20-23: the issue form uses an
invalid "render: Text" (capital T) for a textarea which GitHub Issue Forms does
not recognize; change it to a valid lowercase language like "text" or remove the
render key entirely so the textarea defaults correctly; update the file to use
"render: text" or omit the render field and verify the textarea accepts
free-form input.
| - 'www' | ||
| - '*.md' |
There was a problem hiding this comment.
Fix path filters: current globs won’t match nested files
- 'www' won’t match changes under www/**.
- '*.md' only matches repo-root files.
Use recursive globs.
on:
pull_request:
paths:
- - 'www'
- - '*.md'
+ - 'www/**'
+ - '**/*.md'📝 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.
| - 'www' | |
| - '*.md' | |
| on: | |
| pull_request: | |
| paths: | |
| - 'www/**' | |
| - '**/*.md' |
🤖 Prompt for AI Agents
In .github/workflows/site-pr.yml around lines 6 to 7, the path filters use
non-recursive globs ('www' and '*.md') that won't match nested files; update
them to recursive globs so changes under directories are detected — replace
'www' with 'www/**' to include all files under the www tree and replace '*.md'
with '**/*.md' to include markdown files in any subdirectory.
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v3 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update actions/checkout to v5 (Node 24 runtime)
v3 is deprecated on current runners; bump to v5 to avoid runtime issues. (github.com)
- uses: actions/checkout@v3
+ uses: actions/checkout@v5📝 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.
| uses: actions/checkout@v3 | |
| uses: actions/checkout@v5 |
🧰 Tools
🪛 actionlint (1.7.7)
14-14: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
.github/workflows/site-pr.yml around line 14: the workflow uses
actions/checkout@v3 which is deprecated on current runners; update the action
reference to actions/checkout@v5 to match Node 24 runtime support and avoid
runtime issues, and run a quick CI test to ensure no breaking changes from the
new major version.
| uses: actions/checkout@v3 | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v2 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update Buildx action to v3
docker/setup-buildx-action@v3 is current and required for modern cache APIs/builders. (docs.docker.com)
- uses: docker/setup-buildx-action@v2
+ uses: docker/setup-buildx-action@v3📝 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.
| uses: docker/setup-buildx-action@v2 | |
| uses: docker/setup-buildx-action@v3 |
🧰 Tools
🪛 actionlint (1.7.7)
17-17: the runner of "docker/setup-buildx-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
.github/workflows/site-pr.yml around line 17: the workflow pins
docker/setup-buildx-action to v2 which is outdated; update the action reference
to docker/setup-buildx-action@v3 to use the current Buildx features and modern
cache APIs/builders. Modify the workflow step to reference v3 (ensure no other
incompatible inputs are present) and run a quick CI check to confirm the new
major version does not break other steps.
| return err | ||
| } | ||
| command := strings.Join(args[1:], " ") | ||
| s.Commands[args[0]] = append(s.Commands[args[0]], command) |
There was a problem hiding this comment.
Initialize s.Commands to avoid panic
s.Commands may be nil; writing to a nil map will panic. Guard before append.
- s.Commands[args[0]] = append(s.Commands[args[0]], command)
+ if s.Commands == nil {
+ s.Commands = make(map[string][]string)
+ }
+ s.Commands[args[0]] = append(s.Commands[args[0]], command)📝 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.
| s.Commands[args[0]] = append(s.Commands[args[0]], command) | |
| if s.Commands == nil { | |
| s.Commands = make(map[string][]string) | |
| } | |
| s.Commands[args[0]] = append(s.Commands[args[0]], command) |
🤖 Prompt for AI Agents
In cmd/cmds_add.go around line 24, s.Commands may be nil and appending to a nil
map entry will panic; before appending check if s.Commands is nil and initialize
it with make(map[string][]CommandType) (use the correct element type), and
optionally ensure s.Commands[args[0]] exists by appending to the slice after map
initialization so the append operation is safe.
| func (s *Storage) DeleteWithPathPrefix(path string) error { | ||
| return s.back.DeleteWithPathPrefix(path) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure all backends implement the new interface method
Additions to StorageBackend break implementers until updated. Verify all implementations have DeleteWithPathPrefix.
🏁 Script executed:
#!/bin/sh
# List definitions and call sites to confirm coverage
rg -nP --no-ignore 'DeleteWithPathPrefix\s*\(' -C2Length of output: 977
Fix: ensure DeleteWithPathPrefix is implemented or made optional
- Evidence: share/storage.go defines the method (interface at line 18; wrapper at 123–125), storage/bolt/share.go implements it (line 79), and http/resource.go calls it (line 145). No other implementations found.
- Action: either implement DeleteWithPathPrefix(path string) error on every storage backend, or change the wrapper to call an optional interface via type assertion and handle absence gracefully.
🤖 Prompt for AI Agents
In share/storage.go around lines 123 to 125, the wrapper unconditionally
forwards DeleteWithPathPrefix(path string) error to s.back but not all storage
backends implement it; either add DeleteWithPathPrefix to every backend
(implement the method in storage/bolt/share.go and any other backend) so the
interface is satisfied, or change the wrapper to treat it as optional by
type-asserting s.back to an interface that declares DeleteWithPathPrefix and, if
the assertion fails, return a clear nil/no-op or a defined error; update callers
(e.g., http/resource.go) to handle the returned result consistently.
| docs/LICENSE | ||
| docs/code-of-conduct.md | ||
| docs/contributing.md | ||
| docs/security.md | ||
| docs/changelog.md | ||
| public |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm MkDocs output dir matches ignored “public/”
If site_dir isn’t set to public in www/mkdocs.yml, you may want to ignore site/ instead.
🏁 Script executed:
#!/bin/bash
# Verify mkdocs site output directory
if fd mkdocs.yml www -x awk '/site_dir:/ {print FILENAME ": " $0}'; then
echo "Check that site_dir == public"
else
echo "mkdocs.yml not found under www/"
fiLength of output: 29
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=$(find www -type f -name 'mkdocs.yml' 2>/dev/null || true)
if [ -z "$files" ]; then
echo "mkdocs.yml not found under www/"
exit 0
fi
for f in $files; do
echo "=== $f ==="
grep -nE '^\s*site_dir\s*:' "$f" || {
echo "No site_dir setting found; printing the file for context:"
sed -n '1,200p' "$f"
}
doneLength of output: 2438
Confirm MkDocs output dir matches ignored "public/"
www/mkdocs.yml does not set site_dir — MkDocs defaults to "site". Either add site_dir: public to www/mkdocs.yml or change www/.gitignore to ignore site/ (not public/). Location: www/.gitignore (lines 1–6); www/mkdocs.yml.
🤖 Prompt for AI Agents
In www/.gitignore around lines 1–6 the repo ignores "public/" but www/mkdocs.yml
does not set site_dir (MkDocs defaults to "site"), so update to match: either
add "site_dir: public" to www/mkdocs.yml to make MkDocs output to public/, or
change www/.gitignore to ignore "site/" instead of "public/"; pick one option
and apply consistently so the generated site directory is correctly ignored.
| - **Branding Folder**: directory which can contain two items: | ||
| - `custom.css`, containing a global stylesheet to apply to all users. | ||
| - `img`, a directory which can replace all the [default logotypes](https://github.com/filebrowser/filebrowser/tree/master/frontend/public/img) from the application. | ||
|
|
||
| This can be configured by the administrator user, under **Settings → Global Settings**. You can also update the configuration directly using the CLI: |
There was a problem hiding this comment.
Fix list indentation under “Branding Folder”.
Conform to MD007 (2-space indent).
- - `custom.css`, containing a global stylesheet to apply to all users.
- - `img`, a directory which can replace all the [default logotypes](https://github.com/filebrowser/filebrowser/tree/master/frontend/public/img) from the application.
+ - `custom.css`, containing a global stylesheet to apply to all users.
+ - `img`, a directory which can replace all the [default logotypes](https://github.com/filebrowser/filebrowser/tree/master/frontend/public/img) from the application.📝 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.
| - **Branding Folder**: directory which can contain two items: | |
| - `custom.css`, containing a global stylesheet to apply to all users. | |
| - `img`, a directory which can replace all the [default logotypes](https://github.com/filebrowser/filebrowser/tree/master/frontend/public/img) from the application. | |
| This can be configured by the administrator user, under **Settings → Global Settings**. You can also update the configuration directly using the CLI: | |
| - **Branding Folder**: directory which can contain two items: | |
| - `custom.css`, containing a global stylesheet to apply to all users. | |
| - `img`, a directory which can replace all the [default logotypes](https://github.com/filebrowser/filebrowser/tree/master/frontend/public/img) from the application. | |
| This can be configured by the administrator user, under **Settings → Global Settings**. You can also update the configuration directly using the CLI: |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
13-13: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In www/docs/configuration.md around lines 12 to 16, the nested list under
"Branding Folder" uses incorrect indentation; adjust the list items to conform
to MD007 by using 2-space indentation for the sub-items (e.g., two spaces before
the hyphen for `custom.css` and `img`) so the nested bullet list is properly
aligned and recognized as a sub-list.
|
|
||
| We also provide a no authentication mechanism for users that want to use File Browser privately such in a home network. By setting this authentication method, the user with **id 1** will be used as the default users. Creating more users won't have any effect. | ||
|
|
||
| ```sh | ||
| filebrowser config set --auth.method=noauth | ||
| ``` |
There was a problem hiding this comment.
Grammar fixes in “No Authentication”.
-We also provide a no authentication mechanism for users that want to use File Browser privately such in a home network. By setting this authentication method, the user with **id 1** will be used as the default users. Creating more users won't have any effect.
+We also provide a no-authentication mechanism for users who want to use File Browser privately, such as on a home network. With this method, the user with **id 1** will be used as the default user. Creating more users won't have any effect.📝 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.
| We also provide a no authentication mechanism for users that want to use File Browser privately such in a home network. By setting this authentication method, the user with **id 1** will be used as the default users. Creating more users won't have any effect. | |
| ```sh | |
| filebrowser config set --auth.method=noauth | |
| ``` | |
| We also provide a no-authentication mechanism for users who want to use File Browser privately, such as on a home network. With this method, the user with **id 1** will be used as the default user. Creating more users won't have any effect. | |
🤖 Prompt for AI Agents
In www/docs/configuration.md around lines 90 to 95, fix grammar in the "No
Authentication" paragraph: change "We also provide a no authentication mechanism
for users that want to use File Browser privately such in a home network." to
"We also provide a no-authentication mechanism for users who want to use File
Browser privately, such as on a home network." and change "the user with **id
1** will be used as the default users." to "the user with **id 1** will be used
as the default user." Keep the example command unchanged.
| datepattern = `^%%Y\/%%m\/%%d %%H:%%M:%%S` | ||
| failregex = `\/api\/login: 403 <HOST> *` | ||
| ``` |
There was a problem hiding this comment.
Backticks in fail2ban patterns break the filter.
Remove the backticks; they will be interpreted literally and prevent matches.
Apply:
-datepattern = `^%%Y\/%%m\/%%d %%H:%%M:%%S`
-failregex = `\/api\/login: 403 <HOST> *`
+datepattern = ^%%Y\/%%m\/%%d\ %%H:%%M:%%S
+failregex = \/api\/login:\ 403\ <HOST>\ .*📝 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.
| datepattern = `^%%Y\/%%m\/%%d %%H:%%M:%%S` | |
| failregex = `\/api\/login: 403 <HOST> *` | |
| ``` | |
| datepattern = ^%%Y\/%%m\/%%d\ %%H:%%M:%%S | |
| failregex = \/api\/login:\ 403\ <HOST>\ .* |
🤖 Prompt for AI Agents
In www/docs/deployment.md around lines 14 to 16, the fail2ban patterns are
wrapped in backticks which are treated as literal characters and break matching;
remove the surrounding backticks so the lines read with the raw regex strings
(e.g., datepattern = ^%%Y\/%%m\/%%d %%H:%%M:%%S and failregex = \/api\/login:
403 <HOST> *), ensuring the patterns are unquoted and will be interpreted as
regex by fail2ban.
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v2 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update Buildx action version to v3 to match current runners.
actionlint flags v2 as too old for GH runners.
- - name: Set up Docker Buildx
- uses: docker/setup-buildx-action@v2
+ - name: Set up Docker Buildx
+ uses: docker/setup-buildx-action@v3📝 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.
| - name: Set up Docker Buildx | |
| uses: docker/setup-buildx-action@v2 | |
| - name: Set up Docker Buildx | |
| uses: docker/setup-buildx-action@v3 |
🧰 Tools
🪛 actionlint (1.7.7)
21-21: the runner of "docker/setup-buildx-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In .github/workflows/site-publish.yml around lines 20 to 22, the workflow
currently references docker/setup-buildx-action@v2 which actionlint flags as
outdated for current GH runners; update the action reference to
docker/setup-buildx-action@v3 by changing the uses entry to the v3 tag (and
optionally run a quick workflow lint or dry run to confirm compatibility).
| i, err := strconv.Atoi(args[1]) | ||
| checkErr(err) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| f := i | ||
| if len(args) == 3 { | ||
| f, err = strconv.Atoi(args[2]) | ||
| checkErr(err) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| s.Commands[evt] = append(s.Commands[evt][:i], s.Commands[evt][f+1:]...) |
There was a problem hiding this comment.
Bounds checks missing for i/f → possible panic and incorrect removals.
Current slice operation can panic on out-of-range or negative indices and can produce wrong results when index_end < index. Validate event existence and indices before slicing.
@@
- i, err := strconv.Atoi(args[1])
+ i, err := strconv.Atoi(args[1])
if err != nil {
return err
}
f := i
if len(args) == 3 {
f, err = strconv.Atoi(args[2])
if err != nil {
return err
}
}
- s.Commands[evt] = append(s.Commands[evt][:i], s.Commands[evt][f+1:]...)
+ cmds, ok := s.Commands[evt]
+ if !ok {
+ return fmt.Errorf("unknown event %q", evt)
+ }
+ if i < 0 || i >= len(cmds) {
+ return fmt.Errorf("index %d out of range [0,%d)", i, len(cmds))
+ }
+ if f < 0 || f >= len(cmds) {
+ return fmt.Errorf("index_end %d out of range [0,%d)", f, len(cmds))
+ }
+ if f < i {
+ return fmt.Errorf("index_end (%d) must be >= index (%d)", f, i)
+ }
+ s.Commands[evt] = append(cmds[:i], cmds[f+1:]...)Add required import outside the changed hunk:
import (
"fmt"
"strconv"
"github.com/spf13/cobra"
)🤖 Prompt for AI Agents
In cmd/cmds_rm.go around lines 45 to 57, the slice removal uses indices i and f
without validation which can panic or remove incorrect ranges; first ensure the
event key exists in s.Commands and that i and f are non-negative and within [0,
len(s.Commands[evt])-1], and if len(args)==2 set f=i; after parsing both indices
verify i <= f (swap or return an error if not), then perform the slice removal
using validated bounds; return descriptive errors using fmt.Errorf for any
out-of-range or missing-event conditions and add the required import block
including "fmt" alongside the existing "strconv" and "github.com/spf13/cobra".
| rawAuther = cleanUpInterfaceMap(file.Auther.(map[interface{}]interface{})) | ||
| } else { | ||
| rawAuther = file.Auther | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Defensive type assertion for YAML auther block.
file.Auther.(map[interface{}]interface{}) will panic if auther is missing or not a map. Guard the assertion and return a useful error.
- if filepath.Ext(args[0]) != ".json" {
- rawAuther = cleanUpInterfaceMap(file.Auther.(map[interface{}]interface{}))
+ if filepath.Ext(args[0]) != ".json" {
+ m, ok := file.Auther.(map[interface{}]interface{})
+ if !ok {
+ return errors.New("invalid or missing 'auther' section in config")
+ }
+ rawAuther = cleanUpInterfaceMap(m)📝 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.
| rawAuther = cleanUpInterfaceMap(file.Auther.(map[interface{}]interface{})) | |
| } else { | |
| rawAuther = file.Auther | |
| } | |
| m, ok := file.Auther.(map[interface{}]interface{}) | |
| if !ok { | |
| return errors.New("invalid or missing 'auther' section in config") | |
| } | |
| rawAuther = cleanUpInterfaceMap(m) | |
| } else { | |
| rawAuther = file.Auther | |
| } |
🤖 Prompt for AI Agents
In cmd/config_import.go around lines 69 to 73, the direct type assertion
file.Auther.(map[interface{}]interface{}) can panic if Auther is nil or not a
map; change it to perform a safe type assertion (e.g., check for nil and use the
comma-ok form), return a helpful error when Auther is missing or of the wrong
type, and only call cleanUpInterfaceMap when the assertion succeeds; ensure the
function returns or propagates the error instead of panicking.
| var a interface{} | ||
| a, autherErr = getAuther(&auth.HookAuth{}, rawAuther) | ||
| auther = a.(*auth.HookAuth) | ||
| default: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unify HookAuth sample to non-pointer to avoid pointer-to-pointer pitfalls.
With the fixed getAuther, pass the non-pointer sample for consistency with other cases.
- a, autherErr = getAuther(&auth.HookAuth{}, rawAuther)
+ a, autherErr = getAuther(auth.HookAuth{}, rawAuther)📝 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.
| var a interface{} | |
| a, autherErr = getAuther(&auth.HookAuth{}, rawAuther) | |
| auther = a.(*auth.HookAuth) | |
| default: | |
| var a interface{} | |
| a, autherErr = getAuther(auth.HookAuth{}, rawAuther) | |
| auther = a.(*auth.HookAuth) | |
| default: |
🤖 Prompt for AI Agents
In cmd/config_import.go around lines 90 to 93, the code currently passes a
pointer sample (&auth.HookAuth{}) into getAuther; change the call to pass a
non-pointer sample (auth.HookAuth{}) so it matches the fixed getAuther signature
and avoids pointer-to-pointer issues, keep the returned value assertion as
auther = a.(*auth.HookAuth) and preserve error handling.
| func getAuther(sample auth.Auther, data interface{}) (interface{}, error) { | ||
| authType := reflect.TypeOf(sample) | ||
| auther := reflect.New(authType).Interface() | ||
| bytes, err := json.Marshal(data) | ||
| checkErr(err) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| err = json.Unmarshal(bytes, &auther) | ||
| checkErr(err) | ||
| return auther | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return auther, nil | ||
| } |
There was a problem hiding this comment.
getAuther unmarshalling into &interface{} is incorrect and will break type assertions.
json.Unmarshal(bytes, &auther) sets the interface itself, not the allocated target. Also, passing a pointer sample (HookAuth) yields a pointer-to-pointer. Allocate the concrete type correctly and unmarshal into that pointer.
-func getAuther(sample auth.Auther, data interface{}) (interface{}, error) {
- authType := reflect.TypeOf(sample)
- auther := reflect.New(authType).Interface()
- bytes, err := json.Marshal(data)
- if err != nil {
- return nil, err
- }
- err = json.Unmarshal(bytes, &auther)
- if err != nil {
- return nil, err
- }
- return auther, nil
-}
+func getAuther(sample auth.Auther, data interface{}) (interface{}, error) {
+ authType := reflect.TypeOf(sample)
+ // Always unmarshal into a pointer to the concrete struct, regardless of sample kind.
+ if authType.Kind() == reflect.Pointer {
+ authType = authType.Elem()
+ }
+ target := reflect.New(authType).Interface() // *Concrete
+
+ b, err := json.Marshal(data)
+ if err != nil {
+ return nil, err
+ }
+ if err := json.Unmarshal(b, target); err != nil {
+ return nil, err
+ }
+ return target, nil
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In cmd/config_import.go around lines 110–122, the current code marshals data
then calls json.Unmarshal(bytes, &auther) which writes into the interface
variable itself (and yields a pointer-to-pointer when sample is already a
pointer); instead, determine the concrete target type from sample (use
reflect.TypeOf(sample) and if it's a pointer use Elem() to get the concrete
type), allocate a new instance via reflect.New(concreteType) to get a proper
pointer to the concrete value, unmarshal into that pointer (not &auther), and
return the concrete instance (or its pointer matching the sample's pointer-ness)
so downstream type assertions work correctly.
| req.Data.Password, err = users.ValidateAndHashPwd(req.Data.Password, d.settings.MinimumPasswordLength) | ||
| if err != nil { | ||
| return http.StatusInternalServerError, err | ||
| return http.StatusBadRequest, err | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent empty-password on partial updates.
When updating only the Password field, an empty string would pass if MinimumPasswordLength is 0, enabling empty passwords. Guard explicitly for empty input.
- req.Data.Password, err = users.ValidateAndHashPwd(req.Data.Password, d.settings.MinimumPasswordLength)
+ if req.Data.Password == "" {
+ return http.StatusBadRequest, fbErrors.ErrEmptyPassword
+ }
+ req.Data.Password, err = users.ValidateAndHashPwd(req.Data.Password, d.settings.MinimumPasswordLength)
if err != nil {
return http.StatusBadRequest, err
}📝 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.
| req.Data.Password, err = users.ValidateAndHashPwd(req.Data.Password, d.settings.MinimumPasswordLength) | |
| if err != nil { | |
| return http.StatusInternalServerError, err | |
| return http.StatusBadRequest, err | |
| } | |
| if req.Data.Password == "" { | |
| return http.StatusBadRequest, fbErrors.ErrEmptyPassword | |
| } | |
| req.Data.Password, err = users.ValidateAndHashPwd(req.Data.Password, d.settings.MinimumPasswordLength) | |
| if err != nil { | |
| return http.StatusBadRequest, err | |
| } |
🤖 Prompt for AI Agents
In http/users.go around lines 191-194, the password update path currently calls
ValidateAndHashPwd directly which allows an empty string through when
MinimumPasswordLength is 0; add an explicit guard that if the incoming
req.Data.Password is an empty string (i.e. length == 0) return
http.StatusBadRequest with an appropriate error before calling
ValidateAndHashPwd so empty passwords cannot be set, otherwise proceed to call
ValidateAndHashPwd as before.
| .PHONY: build-release-bin | ||
| build-release-bin: build-frontend | ||
| GO111MODULE=on GOOS=linux GOARCH=amd64 $(go) build -trimpath -ldflags '$(LDFLAGS)' -o bin/filebrowser-$(VERSION) | ||
| tar -C bin -czf "dist/filebrowser-$(VERSION).tar.gz" "filebrowser-$(VERSION)" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure dist exists before creating the tarball.
Without dist/, tar will fail.
build-release-bin: build-frontend
GO111MODULE=on GOOS=linux GOARCH=amd64 $(go) build -trimpath -ldflags '$(LDFLAGS)' -o bin/filebrowser-$(VERSION)
- tar -C bin -czf "dist/filebrowser-$(VERSION).tar.gz" "filebrowser-$(VERSION)"
+ mkdir -p dist
+ tar -C bin -czf "dist/filebrowser-$(VERSION).tar.gz" "filebrowser-$(VERSION)"📝 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.
| .PHONY: build-release-bin | |
| build-release-bin: build-frontend | |
| GO111MODULE=on GOOS=linux GOARCH=amd64 $(go) build -trimpath -ldflags '$(LDFLAGS)' -o bin/filebrowser-$(VERSION) | |
| tar -C bin -czf "dist/filebrowser-$(VERSION).tar.gz" "filebrowser-$(VERSION)" | |
| .PHONY: build-release-bin | |
| build-release-bin: build-frontend | |
| GO111MODULE=on GOOS=linux GOARCH=amd64 $(go) build -trimpath -ldflags '$(LDFLAGS)' -o bin/filebrowser-$(VERSION) | |
| mkdir -p dist | |
| tar -C bin -czf "dist/filebrowser-$(VERSION).tar.gz" "filebrowser-$(VERSION)" |
🤖 Prompt for AI Agents
In Makefile around lines 60 to 64, the build-release-bin target assumes dist/
exists so tar can write the archive; ensure the directory exists before creating
the tarball by adding a step to create dist (e.g., run mkdir -p dist) prior to
the tar command, so the target first builds the binary, then ensures dist is
present, then creates the tar.gz.
| func (s shareBackend) DeleteWithPathPrefix(pathPrefix string) error { | ||
| var links []share.Link | ||
| if err := s.db.Prefix("Path", pathPrefix, &links); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| var err error | ||
| for _, link := range links { | ||
| err = errors.Join(err, s.db.DeleteStruct(&share.Link{Hash: link.Hash})) | ||
| } | ||
| return err | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ignore benign not-found races and optionally batch in a single tx.
Between listing and deletion, links can disappear. Treat storm.ErrNotFound as nil; aggregate only real errors. Optionally wrap in a Bolt update tx for atomicity.
func (s shareBackend) DeleteWithPathPrefix(pathPrefix string) error {
var links []share.Link
if err := s.db.Prefix("Path", pathPrefix, &links); err != nil {
return err
}
- var err error
+ var err error
for _, link := range links {
- err = errors.Join(err, s.db.DeleteStruct(&share.Link{Hash: link.Hash}))
+ if delErr := s.db.DeleteStruct(&share.Link{Hash: link.Hash}); delErr != nil && !errors.Is(delErr, storm.ErrNotFound) {
+ err = errors.Join(err, delErr)
+ }
}
return err
}📝 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.
| func (s shareBackend) DeleteWithPathPrefix(pathPrefix string) error { | |
| var links []share.Link | |
| if err := s.db.Prefix("Path", pathPrefix, &links); err != nil { | |
| return err | |
| } | |
| var err error | |
| for _, link := range links { | |
| err = errors.Join(err, s.db.DeleteStruct(&share.Link{Hash: link.Hash})) | |
| } | |
| return err | |
| } | |
| func (s shareBackend) DeleteWithPathPrefix(pathPrefix string) error { | |
| var links []share.Link | |
| if err := s.db.Prefix("Path", pathPrefix, &links); err != nil { | |
| return err | |
| } | |
| var err error | |
| for _, link := range links { | |
| if delErr := s.db.DeleteStruct(&share.Link{Hash: link.Hash}); delErr != nil && !errors.Is(delErr, storm.ErrNotFound) { | |
| err = errors.Join(err, delErr) | |
| } | |
| } | |
| return err | |
| } |
🤖 Prompt for AI Agents
In storage/bolt/share.go around lines 79 to 90, the DeleteWithPathPrefix
function currently treats any deletion error as fatal even when a link
disappeared between listing and deletion; modify it to ignore storm.ErrNotFound
(treat it as nil) when calling s.db.DeleteStruct and only aggregate non-NotFound
errors (e.g., via errors.Join but skipping err==storm.ErrNotFound), and
optionally perform the deletions inside a single Bolt update transaction
(s.db.Bolt.Update) to make the operation atomic.
| plugins: | ||
| - search: | ||
| separator: '[\\s\\-\\/]+' | ||
| - offline | ||
| - minify: | ||
| minify_html: true | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify plugin/extension availability in the docs build image
offline, minify, and callouts-related extensions require corresponding packages; ensure they’re pinned and installed in CI/container.
Also applies to: 61-89
🏁 Script executed:
#!/bin/bash
# Check for mkdocs deps declarations
fd -a -t f -E node_modules -E vendor -E dist -E build 'requirements*.txt|pyproject.toml|Pipfile|Dockerfile*|Makefile|.github/workflows/*.yml' \
| xargs -I{} sh -c 'echo "== {} =="; sed -n "1,200p" "{}"'
# Look for required packages
rg -n 'mkdocs-material|mkdocs-minify|mkdocs-offline|pymdown-extensions|material-extensions|callouts' -SLength of output: 5776
Add & pin the missing MkDocs "offline" plugin; pin all MkDocs plugin versions
- Action: Add the package that provides the "offline" plugin to www/requirements.txt and pin exact versions for all entries (currently: mkdocs-material, mkdocs-minify-plugin, mkdocs-redirects, markdown-callouts).
- Verify: www/Dockerfile (and CI) installs that requirements.txt (www/Dockerfile already runs pip install -r requirements.txt).
🤖 Prompt for AI Agents
In www/mkdocs.yml around lines 54-60 the "offline" plugin is referenced but not
provided/pinned in www/requirements.txt; update www/requirements.txt to add the
package that provides the offline plugin (e.g., mkdocs-offline or the project's
official plugin package) and pin exact versions for all MkDocs-related packages
(mkdocs-material, mkdocs-minify-plugin, mkdocs-redirects, markdown-callouts and
the offline plugin) using fixed version strings; ensure the pinned requirements
file is the one installed by www/Dockerfile and CI (verify pip install -r
requirements.txt is used) and update any CI/job images if they install a
different requirements file.
|
|
||
| plugins: | ||
| - search: | ||
| separator: '[\\s\\-\\/]+' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix regex escaping for search separator
Single-quote YAML doesn’t process backslashes; the current value double-escapes and likely breaks tokenization.
Apply:
- separator: '[\\s\\-\\/]+'
+ separator: '[\s\-\/]+'📝 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.
| separator: '[\\s\\-\\/]+' | |
| separator: '[\s\-\/]+' |
🤖 Prompt for AI Agents
In www/mkdocs.yml around line 56, the separator regex is single-quoted and
currently double-escapes backslashes incorrectly; change the YAML string to use
double quotes and provide the regex with proper YAML escaping so the pattern
becomes a quoted double-quoted string containing single backslash escapes for
\s, \- and \/ (i.e. use double quotes and escape each backslash once for YAML).
| "copyDownloadLinkToClipboard": "Copy download link to clipboard", | ||
| "create": "Crear", |
There was a problem hiding this comment.
English strings in ES locale (buttons).
- "copyDownloadLinkToClipboard": "Copy download link to clipboard",
+ "copyDownloadLinkToClipboard": "Copiar enlace de descarga al portapapeles",
- "fullScreen": "Toggle full screen",
+ "fullScreen": "Cambiar a pantalla completa",
- "preview": "Preview",
+ "preview": "Vista previa",
- "discardChanges": "Discard"
+ "discardChanges": "Descartar cambios"Also applies to: 19-20, 31-32, 49-51
🤖 Prompt for AI Agents
In frontend/src/i18n/es_CO.json around lines 11-12 (and also fix occurrences at
19-20, 31-32, 49-51), there are English strings in the Spanish locale; replace
them with proper Spanish translations (e.g., change
"copyDownloadLinkToClipboard" value to "Copiar enlace de descarga al
portapapeles" and "create" value to "Crear" if not already correct), and review
the mentioned line ranges to translate any remaining English button labels into
concise Colombian Spanish equivalents so all keys in this file are fully
localized and consistent.
1 participant
Summary by CodeRabbit
New Features
Improvements
Documentation
Build/CI