diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md new file mode 100644 index 00000000..3c5db23a --- /dev/null +++ b/.claude/CLAUDE.md @@ -0,0 +1,132 @@ +# CLAUDE.md + +Guidance for Claude when working in this repository (File Browser, `filebrowser/filebrowser`). + +## Repo orientation + +- Go backend (`github.com/filebrowser/filebrowser/v2`, ecosystem `go`) + Vue frontend under `frontend/`. +- The project is in **maintenance-only mode** (see `SECURITY.md`). Prefer small, surgical, well-tested changes. +- Version scheme: `v2.63.x`. Conventional-commit messages (`fix(scope): …`, `feat: …`, `chore: …`). +- Verify with `go build ./...`, `go vet ./...`, `go test ./...`. Reuse existing test harnesses (e.g. `signToken`, `scopedUserStorage`, `handle`, `customFSUser`, `mockUserStore`). + +--- + +# Handling security advisories + +Use this playbook when asked to triage, verify, fix, or manage GitHub security advisories. +All advisory state lives on GitHub and is driven with the `gh` CLI. States are +`triage → draft → published`, plus `closed`. + +## 1. Fetch + +```bash +# List by state (also: published, draft, closed) +gh api '/repos/filebrowser/filebrowser/security-advisories?state=triage&per_page=100' \ + --jq '.[] | {ghsa_id, severity, summary, state}' + +# Full report for one advisory (read .summary and .description) +gh api /repos/filebrowser/filebrowser/security-advisories/GHSA-xxxx-xxxx-xxxx \ + --jq '.summary, "---", .description' +``` + +Always pull the **published** set too — you need it to dedup against. + +## 2. Verify each report — do NOT trust the report text + +Read the actual source **at HEAD** and reach one verdict per advisory: + +- **CONFIRMED** — defect exists at HEAD. Quote the exact `file:line`. +- **FIXED** — already patched (find the fix commit). +- **FALSE / NOT APPLICABLE** — claim is wrong, or targets a different project. +- **NOT EXPLOITABLE** — pattern exists but no code path can reach the precondition. +- **DUPLICATE** — of a published advisory, or of another triage advisory. + +Common traps — check each before accepting a report: + +- **"Incomplete fix of a prior advisory."** Read the original fix commit and confirm the *specific* + sibling code path is actually still unguarded — a prior fix may already cover a related path. +- **Wrong project.** Confirm the referenced files, symbols, and endpoints exist in this repo; reports + sometimes describe a fork. If the cited code isn't here, close as not applicable. +- **"Legacy / upgraded / imported records are affected."** Trace the field's git history and confirm that + some released version could actually produce such a record before believing it + (`git log -S'Field' -- path`, `git show `, `git tag --contains `). +- **Overlapping reports.** Two triage advisories may share one root cause — consolidate and fix once. +- **Known, intentionally-unaddressed classes.** Some areas are known and tracked but not fixed (see + `SECURITY.md`'s Known Issues); matching reports are duplicates. + +Record, per advisory: verdict, the `file:line` evidence, exploitability preconditions +(default config? platform-specific? auth required?), and the disposition. + +## 3. Fix (one commit per advisory) + +- Branch off `master` first; never commit fixes directly to `master`. +- **One commit per fix**, each referencing the GHSA in the body (`Refs GHSA-xxxx-xxxx-xxxx`). +- When several advisories share a root cause because parallel code paths diverged, **centralize** the logic + (e.g. `settings.CreateUserHome` used by signup, proxy, and hook provisioning) so they cannot drift again. +- Keep it surgical; match surrounding style. + +## 4. Add a regression test per fix + +- Reuse the existing harnesses; add a focused test asserting the fixed behavior (and that the legitimate + path still works). Commit tests alongside or right after the fixes (`test(scope): …`, `Refs GHSA-…`). +- Run `go test ./... && go vet ./...` before finishing. + +## 5. Severity — set a CVSS vector, don't hand-assert + +Set a CVSS v3.1 vector; GitHub derives the score **and** severity from it (overriding the plain `severity` +field). Encode the real preconditions in the vector so the band is defensible: + +- Requires a specific target configuration/platform (e.g. case-insensitive FS) → **AC:H**. +- Unauthenticated vs needs an account → **PR:N / PR:L**. +- Keep it consistent with the **predecessor CVE's** rating (an incomplete-fix follow-up should not outrank + its parent). Bands: `0.1–3.9` low, `4.0–6.9` medium, `7.0–8.9` high, `9.0–10.0` critical. + +```bash +gh api -X PATCH /repos/filebrowser/filebrowser/security-advisories/GHSA-xxxx-xxxx-xxxx \ + -f cvss_vector_string='CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:H/A:H' \ + --jq '{ghsa_id, severity, score: .cvss.score, vector: .cvss.vector_string}' +``` + +## 6. Clean up the title + +Rewrite reporter titles to the concise, sentence-case, backtick-free style of the published advisories +(state the vuln; drop "Incomplete fix of…" prefixes and jargon). The title is the `summary` field: + +```bash +gh api -X PATCH .../security-advisories/GHSA-xxxx-xxxx-xxxx \ + -f summary='Proxy-auth auto-provisioning ignores createUserDir and grants the server root scope' +``` + +## 7. Set affected & patched versions + +The package is always `{ecosystem: "go", name: "github.com/filebrowser/filebrowser/v2"}`. + +- `vulnerable_version_range` — `<= ` (the newest tag; find it with + `git tag --list 'v2.*' --sort=-version:refname | head -1`). +- `patched_versions` — the **next** release that will actually ship the fix. It doesn't exist just + because the branch does; it still has to be cut. **When unsure which version that will be, ask.** + +Replace the placeholder versions below before sending: + +```bash +printf '%s' '{"vulnerabilities":[{"package":{"ecosystem":"go","name":"github.com/filebrowser/filebrowser/v2"},"vulnerable_version_range":"<= ","patched_versions":"","vulnerable_functions":[]}]}' \ + | gh api -X PATCH .../security-advisories/GHSA-xxxx-xxxx-xxxx --input - +``` + +## 8. Move state / close, by disposition + +```bash +gh api -X PATCH .../security-advisories/GHSA-xxxx-xxxx-xxxx -f state=draft # fixed, awaiting release +gh api -X PATCH .../security-advisories/GHSA-xxxx-xxxx-xxxx -f state=closed # duplicate / N-A / not-exploitable +``` + +- **CONFIRMED & fixed** → metadata done → **draft**. Publish after the patched release ships. +- **DUPLICATE / NOT APPLICABLE / NOT EXPLOITABLE** → **closed**. +- **DEFERRED** (confirmed but not yet fixed) → leave in **triage** with a note. +- The REST API **cannot post advisory comments** — replies to reporters (dup notice, evidence, links to + the relevant tracking issue) must be posted manually in the advisory UI. Draft the text for the maintainer. + +## 9. Release & publish + +Push the branch, open a PR, merge, tag/release the `patched_versions` you set, then publish the drafts. +Confirm outward-facing/irreversible advisory actions (close, publish) with the maintainer before doing them. diff --git a/CHANGELOG.md b/CHANGELOG.md index f8c5c235..0dce403a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,90 @@ All notable changes to this project will be documented in this file. See [commit-and-tag-version](https://github.com/absolute-version/commit-and-tag-version) for commit guidelines. +## [2.63.19](https://github.com/filebrowser/filebrowser/compare/v2.63.18...v2.63.19) (2026-07-25) + +### Bug Fixes + +* accessibility and security improvements ([#6033](https://github.com/filebrowser/filebrowser/issues/6033)) ([b21b124](https://github.com/filebrowser/filebrowser/commit/b21b1245ae1b57f031b2f5d787f32a17532402c0)) +* **auth:** isolate auto-provisioned proxy and hook users to their own home ([8ddd3d1](https://github.com/filebrowser/filebrowser/commit/8ddd3d1db9b9f5727d0bf96ea0e7d9a25a8692b4)) +* **http:** delete abandoned TUS uploads through the scoped filesystem ([9bd79c3](https://github.com/filebrowser/filebrowser/commit/9bd79c3aaeb4a55b0e69cf8976c4a258db2f5e06)) +* **http:** enforce declared Upload-Length on TUS uploads ([4daddec](https://github.com/filebrowser/filebrowser/commit/4daddec6f200b03a721197d8c0b4b652c994894e)) +* **http:** enforce download permission on the checksum branch ([6c69b5c](https://github.com/filebrowser/filebrowser/commit/6c69b5cd895e15b29e563200a9a6dfc27b06525e)) +* **http:** run upload hooks for directories ([#6034](https://github.com/filebrowser/filebrowser/issues/6034)) ([9b78324](https://github.com/filebrowser/filebrowser/commit/9b78324d773c790951cc6a97840c4b55f66b5f3d)) +* process --FollowExternalSymlinks ([c05c668](https://github.com/filebrowser/filebrowser/commit/c05c66814891c3cccef394162e428444b53394e4)) +* return error instead of panicking on an unreadable directory during copy ([#6020](https://github.com/filebrowser/filebrowser/issues/6020)) ([ac46cf0](https://github.com/filebrowser/filebrowser/commit/ac46cf06719575477d5125e7472037c204b3702d)) +* **storage:** reject case-folded home directory collisions ([4b8a8d7](https://github.com/filebrowser/filebrowser/commit/4b8a8d72ce554dde378b5091da74aa930ea18327)) +* **upload:** handle encoded path conflicts safely ([#6040](https://github.com/filebrowser/filebrowser/issues/6040)) ([7361d91](https://github.com/filebrowser/filebrowser/commit/7361d91ea2e8cc200a0f40ac84adcd02aedc9ed2)) +## [2.63.18](https://github.com/filebrowser/filebrowser/compare/v2.63.17...v2.63.18) (2026-07-04) + + +### Bug Fixes + +* avoid recursive conflict checks for copy and move ([#6009](https://github.com/filebrowser/filebrowser/issues/6009)) ([dfc2e88](https://github.com/filebrowser/filebrowser/commit/dfc2e887e1a19d54984a0d7e39a2a63caf73ef19)) +* deduplicate PT language ([4470288](https://github.com/filebrowser/filebrowser/commit/4470288ba1828a14453a99348fd22c62aa0b9460)) +* **preview:** keep the EPUB table-of-contents button clear of the header ([#6010](https://github.com/filebrowser/filebrowser/issues/6010)) ([aac2516](https://github.com/filebrowser/filebrowser/commit/aac25166378422135e624e305c410c54a39374fb)) + +## [2.63.17](https://github.com/filebrowser/filebrowser/compare/v2.63.16...v2.63.17) (2026-06-27) + + +### Bug Fixes + +* **auth:** reject signup when normalized home dir collides (GHSA-7rc3-g7h6-22m7) ([883a36f](https://github.com/filebrowser/filebrowser/commit/883a36f02fcb69566a8628cb47f18fdc73348387)) +* match admin share paths by owner scope ([#5992](https://github.com/filebrowser/filebrowser/issues/5992)) ([43a404c](https://github.com/filebrowser/filebrowser/commit/43a404ca69bf25553bfbbb2b446f0f53077c6302)) +* normalize recursive listing paths to forward slashes ([#6003](https://github.com/filebrowser/filebrowser/issues/6003)) ([2472fbc](https://github.com/filebrowser/filebrowser/commit/2472fbcd30502606feb11fbc8b8dc4f3803e6641)) +* preserve SRT subtitle line breaks ([#6002](https://github.com/filebrowser/filebrowser/issues/6002)) ([d9cf2f0](https://github.com/filebrowser/filebrowser/commit/d9cf2f0100d2c4892cad8e339eacca96df1aa5b6)) +* **raw:** neutralize backslashes in archive entry names (GHSA-83xp-526h-j3ww) ([8503ba6](https://github.com/filebrowser/filebrowser/commit/8503ba61ff51d48a7313896483d130eb6a5abfe0)) +* **share:** delete exact directory share on trailing-slash delete (GHSA-pp88-jhwj-5qh5) ([f30fca6](https://github.com/filebrowser/filebrowser/commit/f30fca636c1af9ef401e9a82ff60391cb3db97e1)) +* **share:** stop exposing password hash and bypass token in share API (GHSA-833g-cqhp-h72j) ([ec13054](https://github.com/filebrowser/filebrowser/commit/ec130546713c44cd24556907552ac554c7f809c9)) + +## [2.63.16](https://github.com/filebrowser/filebrowser/compare/v2.63.15...v2.63.16) (2026-06-23) + + +### Bug Fixes + +* dangling symlink, write, delete scope bugs ([64511ce](https://github.com/filebrowser/filebrowser/commit/64511ce45e3be379e965f7f4fb0929a068d5bb81)) +* restore symlink behavior as opt-in followExternalSymlinks ([a106392](https://github.com/filebrowser/filebrowser/commit/a1063925e15ef27f9d5dc26aae371bbf52af608c)) + +## [2.63.15](https://github.com/filebrowser/filebrowser/compare/v2.63.14...v2.63.15) (2026-06-13) + + +### Bug Fixes + +* restore ScopedFs RealPath ([#5986](https://github.com/filebrowser/filebrowser/issues/5986)) ([403d2bb](https://github.com/filebrowser/filebrowser/commit/403d2bbd3357aa57e78745b52dcdad3490901bb3)) + +## [2.63.14](https://github.com/filebrowser/filebrowser/compare/v2.63.13...v2.63.14) (2026-06-07) + + +### Bug Fixes + +* recursive check ([3406d3d](https://github.com/filebrowser/filebrowser/commit/3406d3d7f98dfc3c16e4ff7ff4a87e3bdfe221dd)) + + +### Refactorings + +* ScopedFs to avoid escaping symlinks ([7c2c0a1](https://github.com/filebrowser/filebrowser/commit/7c2c0a11b31b2bb214d741005a0b02b1764208b3)) + +## [2.63.13](https://github.com/filebrowser/filebrowser/compare/v2.63.12...v2.63.13) (2026-06-06) + + +### Bug Fixes + +* copy/move allow overwrite ([a1a514d](https://github.com/filebrowser/filebrowser/commit/a1a514dcbb216d2080412c5354eea1e1fb033050)) + + +### Refactorings + +* cleanup and simplify upload.ts ([5f7311d](https://github.com/filebrowser/filebrowser/commit/5f7311d32437e98d7c14c7b307a4f68109275535)) + +## [2.63.12](https://github.com/filebrowser/filebrowser/compare/v2.63.11...v2.63.12) (2026-06-04) + + +### Bug Fixes + +* await copy move conflict detection ([#5978](https://github.com/filebrowser/filebrowser/issues/5978)) ([c1abe8f](https://github.com/filebrowser/filebrowser/commit/c1abe8f561208bf36bde70879d1a15ef9de998fa)) +* keep mobile file sort controls visible ([#5977](https://github.com/filebrowser/filebrowser/issues/5977)) ([0bb2768](https://github.com/filebrowser/filebrowser/commit/0bb2768754d11b865d68e72dcd7cebb232a6308a)) +* skip inaccessible children when listing directories ([#5958](https://github.com/filebrowser/filebrowser/issues/5958)) ([7b7ff8a](https://github.com/filebrowser/filebrowser/commit/7b7ff8ae8f97393b2e6ae6e061c1f780077c32b6)) + ## [2.63.11](https://github.com/filebrowser/filebrowser/compare/v2.63.10...v2.63.11) (2026-06-04) diff --git a/README.md b/README.md index f2ed7957..cc305b56 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,6 @@

[![Build](https://github.com/filebrowser/filebrowser/actions/workflows/ci.yaml/badge.svg)](https://github.com/filebrowser/filebrowser/actions/workflows/ci.yaml) -[![Go Report Card](https://goreportcard.com/badge/github.com/filebrowser/filebrowser/v2)](https://goreportcard.com/report/github.com/filebrowser/filebrowser/v2) [![Version](https://img.shields.io/github/release/filebrowser/filebrowser.svg)](https://github.com/filebrowser/filebrowser/releases/latest) File Browser provides a file managing interface within a specified directory and it can be used to upload, delete, preview and edit your files. It is a **create-your-own-cloud**-kind of software where you can just install it on your server, direct it to a path and access your files through a nice web interface. @@ -19,7 +18,7 @@ This project is a finished product which fulfills its goal: be a single binary w - It can take a while until someone gets back to you. Please be patient. - [Issues](https://github.com/filebrowser/filebrowser/issues) are meant to track bugs. Unrelated issues will be converted into [discussions](https://github.com/filebrowser/filebrowser/discussions). - The priority is triaging issues, addressing security issues and reviewing pull requests meant to solve bugs. -- No new features are planned. Pull requests for new features are not guaranteed to be reviewed. +- No new features are planned. Pull requests for new features will not be reviewed. Please read [@hacdias' personal reflection](https://hacdias.com/2026/03/11/filebrowser/) on the project status. diff --git a/SECURITY.md b/SECURITY.md index 490a9bea..4d8bb2a8 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -2,25 +2,32 @@ ## Supported Versions -Use this section to tell people about which versions of your project are -currently being supported with security updates. +| Version | Supported | +| ------- | --------- | +| 2.x | ✅ | +| < 2.0 | ❌ | -| Version | Supported | -| ------- | ------------------ | -| 2.x | :white_check_mark: | -| < 2.0 | :x: | +## Before Reporting + +This project is in maintenance-only mode. To avoid duplicates, first check the [existing advisories](https://github.com/filebrowser/filebrowser/security/advisories) and open issues, and confirm: + +- **It concerns this project, not a fork.** Reports about code, features, or endpoints that don't exist here belong to the relevant fork. +- **It isn't an already-known class** that remains unaddressed: + - Command execution, runner, and hooks (opt-in, disabled by default) — [#5199](https://github.com/filebrowser/filebrowser/issues/5199) + - Session and JWT handling — [#5216](https://github.com/filebrowser/filebrowser/issues/5216) + +Reports covering these are likely to be closed as duplicates. ## Reporting a Vulnerability -Vulnerabilities with critical impact should be reported on the [Security](https://github.com/filebrowser/filebrowser/security) page of this repository, which is a private way of communicating vulnerabilities to maintainers. This project is in maintenance-only mode and it can take a while until someone gets back to you. +- **Critical:** report privately via the [Security](https://github.com/filebrowser/filebrowser/security) page. +- **Non-critical:** open a public issue so the community can help; we'll label it as a security issue. -If it is not a critical vulnerability, please open an issue and we will categorize it as a security issue. By giving visibility, we can get more help from the community at fixing such issues. +Please include, where possible: -When reporting an issue, where possible, please provide at least: +- The commit the issue was found at +- A plaintext proof of concept (no binaries) +- Steps to reproduce +- Recommended remediation, if any -* The commit version the issue was identified at -* A proof of concept (plaintext; no binaries) -* Steps to reproduce -* Your recommended remediation(s), if any. - -The File Browser team is a volunteer-only effort, and may reach back out for clarification. +We're a volunteer effort, so responses can take a while, and we may reach out for clarification. diff --git a/auth/hook.go b/auth/hook.go index 60c75461..e53768cb 100644 --- a/auth/hook.go +++ b/auth/hook.go @@ -68,7 +68,7 @@ func (a *HookAuth) Auth(r *http.Request, usr users.Store, stg *settings.Settings case "block": return nil, os.ErrPermission case "pass": - u, err := a.Users.Get(a.Server.Root, a.Cred.Username) + u, err := a.Users.Get(a.Server.Root, a.Server.FollowExternalSymlinks, a.Cred.Username) if err != nil || !users.CheckPwd(a.Cred.Password, u.Password) { return nil, os.ErrPermission } @@ -129,7 +129,7 @@ func (a *HookAuth) GetValues(s string) { // SaveUser updates the existing user or creates a new one when not found func (a *HookAuth) SaveUser() (*users.User, error) { - u, err := a.Users.Get(a.Server.Root, a.Cred.Username) + u, err := a.Users.Get(a.Server.Root, a.Server.FollowExternalSymlinks, a.Cred.Username) if err != nil && !errors.Is(err, fberrors.ErrNotExist) { return nil, err } @@ -157,15 +157,15 @@ func (a *HookAuth) SaveUser() (*users.User, error) { } u = a.GetUser(d) - userHome, err := a.Settings.MakeUserDir(u.Username, u.Scope, a.Server.Root) - if err != nil { - return nil, fmt.Errorf("user: failed to mkdir user home dir: [%s]", userHome) + // A scope explicitly returned by the hook takes precedence over the + // automatic per-user home directory derivation. + _, explicitScope := a.Fields.Values["user.scope"] + if err := a.Settings.CreateUserHome(u, a.Users, a.Server.Root, explicitScope); err != nil { + return nil, err } - u.Scope = userHome - log.Printf("user: %s, home dir: [%s].", u.Username, userHome) + log.Printf("user: %s, home dir: [%s].", u.Username, u.Scope) - err = a.Users.Save(u) - if err != nil { + if err := a.Users.Save(u); err != nil { return nil, err } } else if p := !users.CheckPwd(a.Cred.Password, u.Password); len(a.Fields.Values) > 1 || p { diff --git a/auth/hook_test.go b/auth/hook_test.go index 4b0112b1..10819759 100644 --- a/auth/hook_test.go +++ b/auth/hook_test.go @@ -5,6 +5,9 @@ import ( "path/filepath" "runtime" "testing" + + "github.com/filebrowser/filebrowser/v2/settings" + "github.com/filebrowser/filebrowser/v2/users" ) // writeHookScript writes a POSIX shell script to a temp file and returns its @@ -86,3 +89,68 @@ fi t.Fatalf("expected action %q, got %q", "auth", action) } } + +// newHookAuth builds a HookAuth for a freshly provisioned user with the given +// parsed hook fields (hook.action=auth is added automatically). +func newHookAuth(store *mockUserStore, s *settings.Settings, srv *settings.Server, username string, fields map[string]string) *HookAuth { + fields["hook.action"] = "auth" + return &HookAuth{ + Users: store, + Settings: s, + Server: srv, + Cred: hookCred{Username: username, Password: "a-strong-password"}, + Fields: hookFields{Values: fields}, + } +} + +// With CreateUserDir enabled and no explicit scope from the hook, a provisioned +// hook user must receive its own home directory rather than the server root. +func TestHookSaveUserCreateUserDirIsolatesScope(t *testing.T) { + t.Parallel() + + store := &mockUserStore{users: make(map[string]*users.User)} + srv := &settings.Server{Root: t.TempDir()} + s := &settings.Settings{ + Key: []byte("key"), + CreateUserDir: true, + UserHomeBasePath: "/users", + Defaults: settings.UserDefaults{ + Scope: ".", + Perm: users.Permissions{Create: true}, + }, + } + + u, err := newHookAuth(store, s, srv, "alice", map[string]string{}).SaveUser() + if err != nil { + t.Fatalf("SaveUser error: %v", err) + } + if u.Scope != "/users/alice" { + t.Errorf("hook user without explicit scope: expected /users/alice, got %q", u.Scope) + } +} + +// A scope explicitly returned by the hook takes precedence over the automatic +// per-user home directory derivation. +func TestHookSaveUserRespectsExplicitScope(t *testing.T) { + t.Parallel() + + store := &mockUserStore{users: make(map[string]*users.User)} + srv := &settings.Server{Root: t.TempDir()} + s := &settings.Settings{ + Key: []byte("key"), + CreateUserDir: true, + UserHomeBasePath: "/users", + Defaults: settings.UserDefaults{ + Scope: ".", + Perm: users.Permissions{Create: true}, + }, + } + + u, err := newHookAuth(store, s, srv, "teamlead", map[string]string{"user.scope": "/shared/team"}).SaveUser() + if err != nil { + t.Fatalf("SaveUser error: %v", err) + } + if u.Scope != "/shared/team" { + t.Errorf("explicit hook scope should win, got %q", u.Scope) + } +} diff --git a/auth/json.go b/auth/json.go index 2284dc7f..be8ad1dc 100644 --- a/auth/json.go +++ b/auth/json.go @@ -55,7 +55,7 @@ func (a JSONAuth) Auth(r *http.Request, usr users.Store, _ *settings.Settings, s } } - u, err := usr.Get(srv.Root, cred.Username) + u, err := usr.Get(srv.Root, srv.FollowExternalSymlinks, cred.Username) hash := dummyHash if err == nil { diff --git a/auth/none.go b/auth/none.go index c9381a83..30ea7129 100644 --- a/auth/none.go +++ b/auth/none.go @@ -15,7 +15,7 @@ type NoAuth struct{} // Auth uses authenticates user 1. func (a NoAuth) Auth(_ *http.Request, usr users.Store, _ *settings.Settings, srv *settings.Server) (*users.User, error) { - return usr.Get(srv.Root, uint(1)) + return usr.Get(srv.Root, srv.FollowExternalSymlinks, uint(1)) } // LoginPage tells that no auth doesn't require a login page. diff --git a/auth/proxy.go b/auth/proxy.go index 57eddd4a..c655f8b6 100644 --- a/auth/proxy.go +++ b/auth/proxy.go @@ -20,7 +20,7 @@ type ProxyAuth struct { // Auth authenticates the user via an HTTP header. func (a ProxyAuth) Auth(r *http.Request, usr users.Store, setting *settings.Settings, srv *settings.Server) (*users.User, error) { username := r.Header.Get(a.Header) - user, err := usr.Get(srv.Root, username) + user, err := usr.Get(srv.Root, srv.FollowExternalSymlinks, username) if errors.Is(err, fberrors.ErrNotExist) { return a.createUser(usr, setting, srv, username) } @@ -50,15 +50,11 @@ func (a ProxyAuth) createUser(usr users.Store, setting *settings.Settings, srv * user.Perm.Execute = false user.Commands = []string{} - var userHome string - userHome, err = setting.MakeUserDir(user.Username, user.Scope, srv.Root) - if err != nil { + if err = setting.CreateUserHome(user, usr, srv.Root, false); err != nil { return nil, err } - user.Scope = userHome - err = usr.Save(user) - if err != nil { + if err = usr.Save(user); err != nil { return nil, err } diff --git a/auth/proxy_test.go b/auth/proxy_test.go index 86fb7102..233e72c8 100644 --- a/auth/proxy_test.go +++ b/auth/proxy_test.go @@ -13,7 +13,7 @@ type mockUserStore struct { users map[string]*users.User } -func (m *mockUserStore) Get(_ string, id interface{}) (*users.User, error) { +func (m *mockUserStore) Get(_ string, _ bool, id interface{}) (*users.User, error) { if v, ok := id.(string); ok { if u, ok := m.users[v]; ok { return u, nil @@ -22,14 +22,15 @@ func (m *mockUserStore) Get(_ string, id interface{}) (*users.User, error) { return nil, fberrors.ErrNotExist } -func (m *mockUserStore) Gets(_ string) ([]*users.User, error) { return nil, nil } -func (m *mockUserStore) Update(_ *users.User, _ ...string) error { return nil } +func (m *mockUserStore) GetByScope(_ string) (*users.User, error) { return nil, fberrors.ErrNotExist } +func (m *mockUserStore) Gets(_ string, _ bool) ([]*users.User, error) { return nil, nil } +func (m *mockUserStore) Update(_ *users.User, _ ...string) error { return nil } func (m *mockUserStore) Save(user *users.User) error { m.users[user.Username] = user return nil } func (m *mockUserStore) Delete(_ interface{}) error { return nil } -func (m *mockUserStore) LastUpdate(_ uint) int64 { return 0 } +func (m *mockUserStore) LastUpdate(_ uint) int64 { return 0 } func TestProxyAuthCreateUserRestrictsDefaults(t *testing.T) { t.Parallel() @@ -77,3 +78,46 @@ func TestProxyAuthCreateUserRestrictsDefaults(t *testing.T) { t.Error("auto-provisioned proxy user should retain Create permission from defaults") } } + +// With CreateUserDir enabled, two distinct proxy-authenticated users must each +// receive their own home directory instead of both inheriting the server root. +func TestProxyAuthCreateUserDirIsolatesScope(t *testing.T) { + t.Parallel() + + store := &mockUserStore{users: make(map[string]*users.User)} + srv := &settings.Server{Root: t.TempDir()} + s := &settings.Settings{ + Key: []byte("key"), + AuthMethod: MethodProxyAuth, + CreateUserDir: true, + UserHomeBasePath: "/users", + Defaults: settings.UserDefaults{ + Scope: ".", + Perm: users.Permissions{Create: true}, + }, + } + + auth := ProxyAuth{Header: "X-Remote-User"} + provision := func(name string) *users.User { + req, _ := http.NewRequest(http.MethodGet, "/", http.NoBody) + req.Header.Set("X-Remote-User", name) + u, err := auth.Auth(req, store, s, srv) + if err != nil { + t.Fatalf("Auth(%q) error: %v", name, err) + } + return u + } + + alice := provision("alice") + bob := provision("bob") + + if alice.Scope == "/" || bob.Scope == "/" { + t.Fatalf("provisioned users inherited the server root: alice=%q bob=%q", alice.Scope, bob.Scope) + } + if alice.Scope == bob.Scope { + t.Fatalf("distinct users must get distinct scopes, both got %q", alice.Scope) + } + if alice.Scope != "/users/alice" { + t.Errorf("expected /users/alice, got %q", alice.Scope) + } +} diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go index e4b45c47..91a9c632 100644 --- a/cmd/cmd_test.go +++ b/cmd/cmd_test.go @@ -5,6 +5,10 @@ import ( "github.com/samber/lo" "github.com/spf13/cobra" + "github.com/spf13/pflag" + + "github.com/filebrowser/filebrowser/v2/auth" + "github.com/filebrowser/filebrowser/v2/settings" ) // TestEnvCollisions ensures that there are no collisions in the produced environment @@ -33,3 +37,25 @@ func testEnvCollisions(t *testing.T, cmd *cobra.Command) { t.Errorf("Found duplicate environment variable keys for command %q: %v", cmd.Name(), duplicates) } } + +// TestGetSettingsFollowExternalSymlinks ensures that the followExternalSymlinks +// flag is persisted to the server config when set via "config set". +func TestGetSettingsFollowExternalSymlinks(t *testing.T) { + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + addConfigFlags(flags) + + if err := flags.Parse([]string{"--followExternalSymlinks"}); err != nil { + t.Fatal(err) + } + + set := &settings.Settings{AuthMethod: auth.MethodJSONAuth} + ser := &settings.Server{} + + if _, err := getSettings(flags, set, ser, &auth.JSONAuth{}, false); err != nil { + t.Fatal(err) + } + + if !ser.FollowExternalSymlinks { + t.Error("expected FollowExternalSymlinks to be persisted as true") + } +} diff --git a/cmd/config.go b/cmd/config.go index e3bb2b86..b292f01d 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -229,6 +229,7 @@ func printSettings(ser *settings.Server, set *settings.Settings, auther auth.Aut fmt.Fprintf(w, "\tThumbnails Enabled:\t%t\n", ser.EnableThumbnails) fmt.Fprintf(w, "\tResize Preview:\t%t\n", ser.ResizePreview) fmt.Fprintf(w, "\tType Detection by Header:\t%t\n", ser.TypeDetectionByHeader) + fmt.Fprintf(w, "\tFollow External Symlinks:\t%t\n", ser.FollowExternalSymlinks) fmt.Fprintln(w, "\nTUS:") fmt.Fprintf(w, "\tChunk size:\t%d\n", set.Tus.ChunkSize) @@ -313,6 +314,8 @@ func getSettings(flags *pflag.FlagSet, set *settings.Settings, ser *settings.Ser case "disableImageResolutionCalc": ser.ImageResolutionCal, err = flags.GetBool(flag.Name) ser.ImageResolutionCal = !ser.ImageResolutionCal + case "followExternalSymlinks": + ser.FollowExternalSymlinks, err = flags.GetBool(flag.Name) // Settings flags from [addConfigFlags] case "signup": diff --git a/cmd/root.go b/cmd/root.go index 13139a7e..1d34f866 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -13,6 +13,7 @@ import ( "os" "os/signal" "path/filepath" + "strings" "syscall" "time" @@ -111,6 +112,7 @@ func addServerFlags(flags *pflag.FlagSet) { flags.Bool("disableExec", true, "disables Command Runner feature") flags.Bool("disableTypeDetectionByHeader", false, "disables type detection by reading file headers") flags.Bool("disableImageResolutionCalc", false, "disables image resolution calculation by reading image files") + flags.Bool("followExternalSymlinks", false, "follow symlinks whose target is outside the user scope (unsafe)") } var rootCmd = &cobra.Command{ @@ -353,6 +355,10 @@ func getServerSettings(v *viper.Viper, st *storage.Storage) (*settings.Server, e server.EnableExec = !v.GetBool("disableExec") } + if v.IsSet("followExternalSymlinks") { + server.FollowExternalSymlinks = v.GetBool("followExternalSymlinks") + } + if isAddrSet && isSocketSet { return nil, errors.New("--socket flag cannot be used with --address, --port, --key nor --cert") } @@ -369,6 +375,25 @@ func getServerSettings(v *viper.Viper, st *storage.Storage) (*settings.Server, e log.Println("WARNING: read https://github.com/filebrowser/filebrowser/issues/5199") } + if server.FollowExternalSymlinks { + log.Println("WARNING: Following external symlinks enabled!") + log.Println("WARNING: Symlinks pointing outside a user's scope will be followed,") + log.Println("WARNING: which can expose files outside that scope. Only enable this if") + log.Println("WARNING: you fully understand and trust the contents of every user scope.") + } + + if set, err := st.Settings.Get(); err == nil && set.Signup { + scope := strings.TrimSpace(set.Defaults.Scope) + scopeIsRoot := scope == "" || scope == "." || scope == "/" + + if !set.CreateUserDir && scopeIsRoot { + log.Println("WARNING: Signup is enabled without createUserDir and the default scope is") + log.Println("WARNING: the server root, so every self-registered user can read, modify and") + log.Println("WARNING: delete all files File Browser serves, including other users' files.") + log.Println("WARNING: Enable createUserDir, or set a default scope other than the root.") + } + } + return server, nil } @@ -446,19 +471,20 @@ func quickSetup(v *viper.Viper, s *storage.Storage) error { } ser := &settings.Server{ - BaseURL: v.GetString("baseURL"), - Port: v.GetString("port"), - Log: v.GetString("log"), - TLSKey: v.GetString("key"), - TLSCert: v.GetString("cert"), - Address: v.GetString("address"), - Root: v.GetString("root"), - TokenExpirationTime: v.GetString("tokenExpirationTime"), - EnableThumbnails: !v.GetBool("disableThumbnails"), - ResizePreview: !v.GetBool("disablePreviewResize"), - EnableExec: !v.GetBool("disableExec"), - TypeDetectionByHeader: !v.GetBool("disableTypeDetectionByHeader"), - ImageResolutionCal: !v.GetBool("disableImageResolutionCalc"), + BaseURL: v.GetString("baseURL"), + Port: v.GetString("port"), + Log: v.GetString("log"), + TLSKey: v.GetString("key"), + TLSCert: v.GetString("cert"), + Address: v.GetString("address"), + Root: v.GetString("root"), + TokenExpirationTime: v.GetString("tokenExpirationTime"), + EnableThumbnails: !v.GetBool("disableThumbnails"), + ResizePreview: !v.GetBool("disablePreviewResize"), + EnableExec: !v.GetBool("disableExec"), + TypeDetectionByHeader: !v.GetBool("disableTypeDetectionByHeader"), + ImageResolutionCal: !v.GetBool("disableImageResolutionCalc"), + FollowExternalSymlinks: v.GetBool("followExternalSymlinks"), } err = s.Settings.SaveServer(ser) diff --git a/cmd/rules.go b/cmd/rules.go index bdb1d1cf..f3e00bb6 100644 --- a/cmd/rules.go +++ b/cmd/rules.go @@ -36,7 +36,7 @@ func runRules(st *storage.Storage, cmd *cobra.Command, usersFn func(*users.User) } if id != nil { var user *users.User - user, err = st.Users.Get("", id) + user, err = st.Users.Get("", false, id) if err != nil { return err } diff --git a/cmd/users_export.go b/cmd/users_export.go index 9bbec6d8..768c381f 100644 --- a/cmd/users_export.go +++ b/cmd/users_export.go @@ -15,7 +15,7 @@ var usersExportCmd = &cobra.Command{ path to the file where you want to write the users.`, Args: jsonYamlArg, RunE: withStore(func(_ *cobra.Command, args []string, st *store) error { - list, err := st.Users.Gets("") + list, err := st.Users.Gets("", false) if err != nil { return err } diff --git a/cmd/users_find.go b/cmd/users_find.go index 09bc8d47..fc91af86 100644 --- a/cmd/users_find.go +++ b/cmd/users_find.go @@ -36,14 +36,14 @@ var findUsers = withStore(func(_ *cobra.Command, args []string, st *store) error if len(args) == 1 { username, id := parseUsernameOrID(args[0]) if username != "" { - user, err = st.Users.Get("", username) + user, err = st.Users.Get("", false, username) } else { - user, err = st.Users.Get("", id) + user, err = st.Users.Get("", false, id) } list = []*users.User{user} } else { - list, err = st.Users.Gets("") + list, err = st.Users.Gets("", false) } if err != nil { diff --git a/cmd/users_import.go b/cmd/users_import.go index 73effca6..0db3704f 100644 --- a/cmd/users_import.go +++ b/cmd/users_import.go @@ -40,7 +40,7 @@ list or set it to 0.`, } for _, user := range list { - err = user.Clean("") + err = user.Clean("", false) if err != nil { return err } @@ -52,7 +52,7 @@ list or set it to 0.`, } if replace { - oldUsers, userImportErr := st.Users.Gets("") + oldUsers, userImportErr := st.Users.Gets("", false) if userImportErr != nil { return userImportErr } @@ -76,7 +76,7 @@ list or set it to 0.`, } for _, user := range list { - onDB, err := st.Users.Get("", user.ID) + onDB, err := st.Users.Get("", false, user.ID) // User exists in DB. if err == nil { @@ -88,7 +88,7 @@ list or set it to 0.`, // with the new username. If there is, print an error and cancel the // operation if user.Username != onDB.Username { - if conflictuous, err := st.Users.Get("", user.Username); err == nil { + if conflictuous, err := st.Users.Get("", false, user.Username); err == nil { return usernameConflictError(user.Username, conflictuous.ID, user.ID) } } diff --git a/cmd/users_update.go b/cmd/users_update.go index e9a484fc..12484342 100644 --- a/cmd/users_update.go +++ b/cmd/users_update.go @@ -43,9 +43,9 @@ options you want to change.`, user *users.User ) if id != 0 { - user, err = st.Users.Get("", id) + user, err = st.Users.Get("", false, id) } else { - user, err = st.Users.Get("", username) + user, err = st.Users.Get("", false, username) } if err != nil { return err diff --git a/files/file.go b/files/file.go index a034abc0..4102c3c4 100644 --- a/files/file.go +++ b/files/file.go @@ -18,13 +18,13 @@ import ( "path" "path/filepath" "regexp" + "sort" "strings" "time" - "github.com/spf13/afero" - fberrors "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/rules" + "github.com/spf13/afero" ) var ( @@ -128,13 +128,6 @@ func stat(opts *FileOptions) (*FileInfo, error) { } } - if file != nil { - ok, scopeErr := WithinScope(opts.Fs, opts.Path) - if scopeErr != nil || !ok { - return nil, os.ErrPermission - } - } - // regular file if file != nil && !file.IsSymlink { return file, nil @@ -172,60 +165,6 @@ func stat(opts *FileOptions) (*FileInfo, error) { return file, nil } -// WithinScope reports whether the on-disk target of p — after resolving any -// symbolic links — stays within the scoped root of fsys. It exists to stop a -// symlink that lives lexically inside a user's scope but points outside it -// from being followed for reads, writes, or shares. -// -// Paths that do not exist yet (e.g. a brand-new file being created) are -// validated against their nearest existing ancestor, so legitimate new files -// are always allowed. For a filesystem that is not scoped with BasePathFs the -// check is a no-op and returns true. -// -// Note: a dangling symlink whose target does not yet exist resolves to its -// containing directory and is therefore allowed; writing through such a link -// could still create a file outside the scope. Callers that create files -// should treat this as best-effort and rely on rejecting existing escaping -// symlinks, which covers the disclosure and overwrite vectors. -func WithinScope(fsys afero.Fs, p string) (bool, error) { - bfs, ok := fsys.(*afero.BasePathFs) - if !ok { - // Not a scoped filesystem; nothing to enforce. - return true, nil - } - - root, err := filepath.EvalSymlinks(afero.FullBaseFsPath(bfs, "/")) - if err != nil { - return false, err - } - - target := afero.FullBaseFsPath(bfs, p) - resolved, err := filepath.EvalSymlinks(target) - for errors.Is(err, fs.ErrNotExist) { - parent := filepath.Dir(target) - if parent == target { - break - } - target = parent - resolved, err = filepath.EvalSymlinks(target) - } - if err != nil { - return false, err - } - - // Compare against root with a trailing separator so a sibling like - // "/srvother" is not treated as being inside "/srv". When root is itself - // the filesystem boundary (e.g. "/"), it already ends in a separator, so - // avoid producing "//" — which no path would match — and accept any path - // under it. - prefix := root - if !strings.HasSuffix(prefix, string(filepath.Separator)) { - prefix += string(filepath.Separator) - } - - return resolved == root || strings.HasPrefix(resolved, prefix), nil -} - // Checksum checksums a given File for a given User, using a specific // algorithm. The checksums data is saved on File object. func (i *FileInfo) Checksum(algo string) error { @@ -452,8 +391,7 @@ func (i *FileInfo) addSubtitle(fPath string) { } func (i *FileInfo) readListing(checker rules.Checker, readHeader bool, calcImgRes bool) error { - afs := &afero.Afero{Fs: i.Fs} - dir, err := afs.ReadDir(i.Path) + dir, err := readDir(i.Fs, i.Path) if err != nil { return err } @@ -475,19 +413,19 @@ func (i *FileInfo) readListing(checker rules.Checker, readHeader bool, calcImgRe isSymlink, isInvalidLink := false, false if IsSymlink(f.Mode()) { isSymlink = true - // A symlink whose on-disk target escapes the scoped root must not be - // followed, otherwise the listing would leak the target's metadata - // (and downstream access) for files outside the user's scope or the - // shared subtree. - if ok, scopeErr := WithinScope(i.Fs, fPath); scopeErr != nil || !ok { - continue - } - // It's a symbolic link. We try to follow it. If it doesn't work, - // we stay with the link information instead of the target's. + // It's a symbolic link. We try to follow it. The scoped filesystem + // refuses to dereference a link whose target escapes the scope + // (permission error); such a link is omitted from the listing + // entirely so it cannot leak the target's metadata. Any other + // failure means a broken link, which we surface as an invalid link + // rather than the target's information. info, err := i.Fs.Stat(fPath) - if err == nil { + switch { + case err == nil: f = info - } else { + case errors.Is(err, os.ErrPermission): + continue + default: isInvalidLink = true } } @@ -535,3 +473,56 @@ func (i *FileInfo) readListing(checker rules.Checker, readHeader bool, calcImgRe i.Listing = listing return nil } + +func readDir(afs afero.Fs, dirname string) ([]os.FileInfo, error) { + dir, err := afero.ReadDir(afs, dirname) + if err == nil { + return dir, nil + } + + dir, fallbackErr := readDirNames(afs, dirname) + if fallbackErr != nil { + return nil, err + } + + return dir, nil +} + +func readDirNames(afs afero.Fs, dirname string) ([]os.FileInfo, error) { + file, err := afs.Open(dirname) + if err != nil { + return nil, err + } + + names, err := file.Readdirnames(-1) + if closeErr := file.Close(); err == nil && closeErr != nil { + err = closeErr + } + if err != nil { + return nil, err + } + + sort.Strings(names) + dir := make([]os.FileInfo, 0, len(names)) + for _, name := range names { + fPath := path.Join(dirname, name) + info, err := lstatIfPossible(afs, fPath) + if err != nil { + log.Printf("Skipping inaccessible file %s: %v", fPath, err) + continue + } + + dir = append(dir, info) + } + + return dir, nil +} + +func lstatIfPossible(afs afero.Fs, name string) (os.FileInfo, error) { + if lstaterFs, ok := afs.(afero.Lstater); ok { + info, _, err := lstaterFs.LstatIfPossible(name) + return info, err + } + + return afs.Stat(name) +} diff --git a/files/file_test.go b/files/file_test.go index d1f647f3..45b114f6 100644 --- a/files/file_test.go +++ b/files/file_test.go @@ -2,60 +2,53 @@ package files import ( "os" + "path" "path/filepath" "testing" "github.com/spf13/afero" ) -func TestWithinScope(t *testing.T) { - t.Run("non-scoped filesystem is a no-op", func(t *testing.T) { - ok, err := WithinScope(afero.NewOsFs(), "/anything") - if err != nil || !ok { - t.Fatalf("expected (true, nil), got (%v, %v)", ok, err) - } - }) - - t.Run("path inside a nested scope is allowed", func(t *testing.T) { +func TestScopedFs(t *testing.T) { + t.Run("path inside scope is allowed", func(t *testing.T) { scope := t.TempDir() if err := os.WriteFile(filepath.Join(scope, "file.txt"), []byte("x"), 0o644); err != nil { t.Fatal(err) } - bfs := afero.NewBasePathFs(afero.NewOsFs(), scope) + fs := NewScopedFs(afero.NewOsFs(), scope) - ok, err := WithinScope(bfs, "/file.txt") - if err != nil || !ok { - t.Fatalf("expected (true, nil), got (%v, %v)", ok, err) + if _, err := fs.Stat("/file.txt"); err != nil { + t.Fatalf("expected in-scope file to be accessible, got %v", err) } }) - t.Run("new file inside scope is allowed", func(t *testing.T) { + t.Run("new file inside scope can be created", func(t *testing.T) { scope := t.TempDir() - bfs := afero.NewBasePathFs(afero.NewOsFs(), scope) + fs := NewScopedFs(afero.NewOsFs(), scope) - ok, err := WithinScope(bfs, "/does-not-exist-yet.txt") - if err != nil || !ok { - t.Fatalf("expected (true, nil), got (%v, %v)", ok, err) + f, err := fs.OpenFile("/does-not-exist-yet.txt", os.O_RDWR|os.O_CREATE, 0o644) + if err != nil { + t.Fatalf("expected to create a new in-scope file, got %v", err) } + _ = f.Close() }) // Regression for #5975: when the scope resolves to the filesystem root, // root+separator used to be "//", which no path matched, so every write // was rejected with os.ErrPermission (HTTP 403). - t.Run("filesystem root scope allows writes", func(t *testing.T) { + t.Run("filesystem root scope allows access", func(t *testing.T) { f := filepath.Join(t.TempDir(), "file.txt") if err := os.WriteFile(f, []byte("x"), 0o644); err != nil { t.Fatal(err) } - bfs := afero.NewBasePathFs(afero.NewOsFs(), "/") + fs := NewScopedFs(afero.NewOsFs(), "/") - ok, err := WithinScope(bfs, f) - if err != nil || !ok { - t.Fatalf("expected (true, nil) for a path under root scope, got (%v, %v)", ok, err) + if _, err := fs.Stat(f); err != nil { + t.Fatalf("expected a path under root scope to be accessible, got %v", err) } }) - t.Run("sibling of a nested scope is rejected", func(t *testing.T) { + t.Run("escaping symlink to a sibling is rejected", func(t *testing.T) { base := t.TempDir() scope := filepath.Join(base, "srv") sibling := filepath.Join(base, "srvother") @@ -64,20 +57,21 @@ func TestWithinScope(t *testing.T) { t.Fatal(err) } } - // A symlink lexically inside the scope pointing at a sibling directory - // must not be followed. - link := filepath.Join(scope, "escape") - if err := os.Symlink(sibling, link); err != nil { + if err := os.WriteFile(filepath.Join(sibling, "secret.txt"), []byte("secret"), 0o644); err != nil { t.Fatal(err) } - bfs := afero.NewBasePathFs(afero.NewOsFs(), scope) - - ok, err := WithinScope(bfs, "/escape") - if err != nil { - t.Fatalf("unexpected error: %v", err) + // A symlink lexically inside the scope pointing at a sibling directory + // must not be followed for reads or stats. + if err := os.Symlink(sibling, filepath.Join(scope, "escape")); err != nil { + t.Skipf("cannot create symlink: %v", err) } - if ok { - t.Fatal("expected escaping symlink to a sibling directory to be rejected") + fs := NewScopedFs(afero.NewOsFs(), scope) + + if _, err := fs.Stat("/escape"); !os.IsPermission(err) { + t.Fatalf("expected stat of escaping symlink to be rejected, got %v", err) + } + if _, err := fs.Open("/escape/secret.txt"); !os.IsPermission(err) { + t.Fatalf("expected read through escaping symlink to be rejected, got %v", err) } }) @@ -92,11 +86,129 @@ func TestWithinScope(t *testing.T) { if err := os.Symlink(filepath.Join(scope, "real"), filepath.Join(scope, "link")); err != nil { t.Skipf("cannot create symlink: %v", err) } - bfs := afero.NewBasePathFs(afero.NewOsFs(), scope) + fs := NewScopedFs(afero.NewOsFs(), scope) - ok, err := WithinScope(bfs, "/link/f.txt") - if err != nil || !ok { - t.Fatalf("expected (true, nil) for an in-scope symlink target, got (%v, %v)", ok, err) + if _, err := fs.Stat("/link/f.txt"); err != nil { + t.Fatalf("expected in-scope symlink target to be accessible, got %v", err) + } + }) + + // Regression for the dangling-symlink write escape (GHSA-8wc8-hf36-mjh9 / + // GHSA-fh54-6rfh-r8f3): a symlink whose target does not exist yet must not be + // followed for writes. Previously within() validated the link's in-scope + // parent directory, so OpenFile(O_CREATE) dereferenced the link and created + // the file at its out-of-scope target. + t.Run("write through a dangling escaping symlink is rejected", func(t *testing.T) { + base := t.TempDir() + scope := filepath.Join(base, "scope") + outside := filepath.Join(base, "outside") + for _, d := range []string{scope, outside} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + outsideTarget := filepath.Join(outside, "created.txt") // does not exist yet + if err := os.Symlink(outsideTarget, filepath.Join(scope, "evil")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + fs := NewScopedFs(afero.NewOsFs(), scope) + + f, err := fs.OpenFile("/evil", os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o644) + if err == nil { + _ = f.Close() + t.Fatal("VULNERABLE: write through a dangling escaping symlink was allowed") + } + if !os.IsPermission(err) { + t.Fatalf("expected permission error, got %v", err) + } + if _, statErr := os.Stat(outsideTarget); statErr == nil { + t.Fatal("VULNERABLE: file was created outside the scope") + } + }) + + // A dangling *relative* symlink that lives under an escaping directory + // symlink must be resolved against the link's real directory, not its lexical + // parent. Otherwise the symlinked ancestor can shift the computed target back + // into scope while the real OS write lands outside it. + t.Run("write through a dangling relative symlink under a symlinked dir is rejected", func(t *testing.T) { + base := t.TempDir() + scope := filepath.Join(base, "scope") + outside := filepath.Join(base, "outside") + for _, d := range []string{scope, outside} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + // An escaping directory symlink inside the scope: /scope/m -> /base/outside. + if err := os.Symlink(outside, filepath.Join(scope, "m")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + // A relative dangling symlink inside the escaping dir whose target, + // resolved against the real directory (/base/outside), is /base/escaped — + // outside the scope. + if err := os.Symlink("../escaped", filepath.Join(outside, "evil")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + fs := NewScopedFs(afero.NewOsFs(), scope) + + f, err := fs.OpenFile("/m/evil", os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o644) + if err == nil { + _ = f.Close() + t.Fatal("VULNERABLE: write through a dangling relative symlink under a symlinked dir was allowed") + } + if !os.IsPermission(err) { + t.Fatalf("expected permission error, got %v", err) + } + if _, statErr := os.Stat(filepath.Join(base, "escaped")); statErr == nil { + t.Fatal("VULNERABLE: file was created outside the scope") + } + }) + + // Regression for the symlink-following delete escape (GHSA-hq4g-mpch-f9vp / + // GHSA-fmm7-x4gx-8jhr): Remove/RemoveAll used to skip guard(), so RemoveAll + // followed a symlinked ancestor escaping the scope and deleted an + // out-of-scope file. + t.Run("RemoveAll through an escaping symlink is rejected", func(t *testing.T) { + base := t.TempDir() + scope := filepath.Join(base, "scope") + outside := filepath.Join(base, "outside") + for _, d := range []string{scope, outside} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + victim := filepath.Join(outside, "victim.txt") + if err := os.WriteFile(victim, []byte("keep"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(scope, "link")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + fs := NewScopedFs(afero.NewOsFs(), scope) + + if err := fs.RemoveAll("/link/victim.txt"); !os.IsPermission(err) { + t.Fatalf("expected RemoveAll through escaping symlink to be rejected, got %v", err) + } + if _, statErr := os.Stat(victim); statErr != nil { + t.Fatalf("VULNERABLE: out-of-scope victim file was deleted: %v", statErr) + } + }) + + // The guard added for the delete escape must not break legitimate deletes of + // in-scope files. + t.Run("RemoveAll of an in-scope file is allowed", func(t *testing.T) { + scope := t.TempDir() + target := filepath.Join(scope, "deleteme.txt") + if err := os.WriteFile(target, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + fs := NewScopedFs(afero.NewOsFs(), scope) + + if err := fs.RemoveAll("/deleteme.txt"); err != nil { + t.Fatalf("expected in-scope RemoveAll to succeed, got %v", err) + } + if _, statErr := os.Stat(target); statErr == nil { + t.Fatal("expected in-scope file to be deleted") } }) } @@ -122,7 +234,7 @@ func TestStatRejectsLinkedAncestorEscape(t *testing.T) { } // Filesystem scoped to the shared directory, as a public share would be. - bfs := afero.NewBasePathFs(afero.NewOsFs(), filepath.Join(scope, "shared")) + bfs := NewScopedFs(afero.NewOsFs(), filepath.Join(scope, "shared")) if _, err := stat(&FileOptions{Fs: bfs, Path: "/link/secret.txt"}); !os.IsPermission(err) { t.Fatalf("expected permission error for linked-ancestor escape, got %v", err) @@ -131,3 +243,105 @@ func TestStatRejectsLinkedAncestorEscape(t *testing.T) { t.Fatalf("expected in-scope file to be served, got %v", err) } } + +type allowAllChecker struct{} + +func (allowAllChecker) Check(string) bool { + return true +} + +type inaccessibleChildFs struct { + afero.Fs + child string +} + +func (fs inaccessibleChildFs) Open(name string) (afero.File, error) { + file, err := fs.Fs.Open(name) + if err != nil { + return nil, err + } + + if path.Clean(name) == "/" { + return inaccessibleChildDir{File: file}, nil + } + + return file, nil +} + +func (fs inaccessibleChildFs) Stat(name string) (os.FileInfo, error) { + if path.Clean(name) == fs.child { + return nil, os.ErrPermission + } + + return fs.Fs.Stat(name) +} + +func (fs inaccessibleChildFs) LstatIfPossible(name string) (os.FileInfo, bool, error) { + if path.Clean(name) == fs.child { + return nil, false, os.ErrPermission + } + + if lstater, ok := fs.Fs.(afero.Lstater); ok { + return lstater.LstatIfPossible(name) + } + + info, err := fs.Fs.Stat(name) + return info, false, err +} + +type inaccessibleChildDir struct { + afero.File +} + +func (dir inaccessibleChildDir) Readdir(int) ([]os.FileInfo, error) { + return nil, os.ErrPermission +} + +func TestReadListingSkipsInaccessibleChildren(t *testing.T) { + memFs := afero.NewMemMapFs() + for _, dir := range []string{"/media", "/proton-mount"} { + if err := memFs.Mkdir(dir, 0o755); err != nil { + t.Fatal(err) + } + } + + file, err := NewFileInfo(&FileOptions{ + Fs: inaccessibleChildFs{Fs: memFs, child: "/proton-mount"}, + Path: "/", + Expand: true, + Checker: allowAllChecker{}, + }) + if err != nil { + t.Fatal(err) + } + + if file.Listing == nil { + t.Fatal("expected root listing") + } + + if got := len(file.Items); got != 1 { + t.Fatalf("expected one accessible child, got %d", got) + } + + if got := file.Items[0].Name; got != "media" { + t.Fatalf("expected accessible child to be listed, got %q", got) + } + + if got := file.NumDirs; got != 1 { + t.Fatalf("expected one listed directory, got %d", got) + } +} + +func TestFileInfoRealPathUsesScopedFsRealPath(t *testing.T) { + root := t.TempDir() + file := &FileInfo{ + Fs: NewScopedFs(afero.NewOsFs(), root), + Path: "/root/downloads", + } + + got := file.RealPath() + want := filepath.Join(root, "root", "downloads") + if got != want { + t.Fatalf("got %q, want %q", got, want) + } +} diff --git a/files/fs_test.go b/files/fs_test.go new file mode 100644 index 00000000..5974b20a --- /dev/null +++ b/files/fs_test.go @@ -0,0 +1,121 @@ +package files + +import ( + "os" + "path/filepath" + "testing" + + "github.com/spf13/afero" +) + +// TestNewFs verifies that NewFs picks the right implementation and that the +// follow-external-symlinks toggle flips whether a symlink pointing outside the +// scope is honored. +func TestNewFs(t *testing.T) { + base := t.TempDir() + scope := filepath.Join(base, "srv") + outside := filepath.Join(base, "outside") + for _, d := range []string{scope, outside} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0o644); err != nil { + t.Fatal(err) + } + // A symlink lexically inside the scope whose target resolves outside it. + if err := os.Symlink(outside, filepath.Join(scope, "escape")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + + t.Run("disabled returns a ScopedFs that rejects the escaping symlink", func(t *testing.T) { + fs := NewFs(afero.NewOsFs(), scope, false) + if _, ok := fs.(*ScopedFs); !ok { + t.Fatalf("expected *ScopedFs, got %T", fs) + } + if _, err := fs.Stat("/escape"); !os.IsPermission(err) { + t.Fatalf("expected stat of escaping symlink to be rejected, got %v", err) + } + }) + + t.Run("enabled returns a BasePathFs that follows the escaping symlink", func(t *testing.T) { + fs := NewFs(afero.NewOsFs(), scope, true) + if _, ok := fs.(*afero.BasePathFs); !ok { + t.Fatalf("expected *afero.BasePathFs, got %T", fs) + } + if _, err := fs.Stat("/escape"); err != nil { + t.Fatalf("expected escaping symlink to be followed, got %v", err) + } + b, err := afero.ReadFile(fs, "/escape/secret.txt") + if err != nil { + t.Fatalf("expected to read through escaping symlink, got %v", err) + } + if string(b) != "secret" { + t.Fatalf("got %q, want %q", b, "secret") + } + + // The link must also appear in a directory listing (the symptom in #5998). + entries, err := afero.ReadDir(fs, "/") + if err != nil { + t.Fatal(err) + } + var found bool + for _, e := range entries { + if e.Name() == "escape" { + found = true + } + } + if !found { + t.Fatal("expected escaping symlink to appear in the listing") + } + }) +} + +// TestBasePath verifies BasePath extracts the underlying *afero.BasePathFs from +// either filesystem NewFs may return, so User.FullPath keeps working. +func TestBasePath(t *testing.T) { + root := t.TempDir() + osFs := afero.NewOsFs() + + for _, tc := range []struct { + name string + followExternal bool + }{ + {"ScopedFs", false}, + {"BasePathFs", true}, + } { + t.Run(tc.name, func(t *testing.T) { + fs := NewFs(osFs, root, tc.followExternal) + base := BasePath(fs) + if base == nil { + t.Fatalf("expected non-nil base for %T", fs) + } + got := afero.FullBaseFsPath(base, "/x") + want := filepath.Join(root, "x") + if got != want { + t.Fatalf("FullBaseFsPath: got %q, want %q", got, want) + } + }) + } + + if got := BasePath(osFs); got != nil { + t.Fatalf("expected nil base for a plain OsFs, got %v", got) + } +} + +// TestFileInfoRealPathUsesBasePathFsRealPath mirrors +// TestFileInfoRealPathUsesScopedFsRealPath for the follow-external-symlinks case, +// where the user filesystem is a bare BasePathFs. +func TestFileInfoRealPathUsesBasePathFsRealPath(t *testing.T) { + root := t.TempDir() + file := &FileInfo{ + Fs: NewFs(afero.NewOsFs(), root, true), + Path: "/root/downloads", + } + + got := file.RealPath() + want := filepath.Join(root, "root", "downloads") + if got != want { + t.Fatalf("got %q, want %q", got, want) + } +} diff --git a/files/scoped.go b/files/scoped.go new file mode 100644 index 00000000..97ce32d0 --- /dev/null +++ b/files/scoped.go @@ -0,0 +1,253 @@ +package files + +import ( + "errors" + "io/fs" + "os" + "path/filepath" + "strings" + "time" + + "github.com/spf13/afero" +) + +// ScopedFs is an afero.Fs that confines every operation to a base directory and +// refuses to follow a symbolic link whose on-disk target resolves outside that +// base. It wraps an *afero.BasePathFs — which already provides the lexical +// confinement — and adds a per-operation scope check on every call that would +// dereference a symlink at the OS layer (open, stat, lstat, chmod, …). +type ScopedFs struct { + base *afero.BasePathFs +} + +var ( + _ afero.Fs = (*ScopedFs)(nil) + _ afero.Lstater = (*ScopedFs)(nil) +) + +// maxSymlinkHops bounds how many dangling symlinks within() will follow before +// giving up, so a pathological chain cannot loop forever. It mirrors the kernel +// MAXSYMLINKS limit; the operation is rejected once the bound is exceeded. +const maxSymlinkHops = 255 + +func NewScopedFs(source afero.Fs, path string) *ScopedFs { + if s, ok := source.(*ScopedFs); ok { + source = s.base + } + return &ScopedFs{base: afero.NewBasePathFs(source, path).(*afero.BasePathFs)} +} + +// NewFs builds a user filesystem rooted at path. When followExternal is true it +// returns a bare BasePathFs, so symlinks whose target resolves outside the scope +// are followed; otherwise it returns a ScopedFs that refuses to follow them. +func NewFs(source afero.Fs, path string, followExternal bool) afero.Fs { + if followExternal { + return afero.NewBasePathFs(source, path) + } + return NewScopedFs(source, path) +} + +// BasePath returns the underlying *afero.BasePathFs of a user filesystem built +// by NewFs, whether it is a *ScopedFs or a bare *afero.BasePathFs, or nil if it +// is neither. +func BasePath(fs afero.Fs) *afero.BasePathFs { + switch f := fs.(type) { + case *ScopedFs: + return f.BasePathFs() + case *afero.BasePathFs: + return f + } + return nil +} + +// BasePathFs returns the underlying *afero.BasePathFs. +func (s *ScopedFs) BasePathFs() *afero.BasePathFs { return s.base } + +// RealPath resolves a scoped path to the real on-disk path by delegating to +// the underlying BasePathFs. This is needed by callers that need the actual +// filesystem path (e.g. disk.UsageWithContext). +func (s *ScopedFs) RealPath(name string) (string, error) { + return s.base.RealPath(name) +} + +// guard returns an error if name's on-disk target resolves outside the scope. +func (s *ScopedFs) guard(name string) error { + ok, err := s.within(name) + if err != nil { + return err + } + if !ok { + return os.ErrPermission + } + return nil +} + +// within reports whether the on-disk target of p — after resolving any symbolic +// links — stays within the scoped root. It exists to stop a symlink that lives +// lexically inside the scope but points outside it from being followed for +// reads, writes, or shares. +// +// Paths that do not exist yet (e.g. a brand-new file being created) are +// validated against their nearest existing ancestor, so legitimate new files +// are always allowed. A dangling symlink — a link whose target does not exist +// yet — is the exception: it is followed to where it points and validated +// there, so a write cannot dereference the link to create a file outside the +// scope. +func (s *ScopedFs) within(p string) (bool, error) { + root, err := filepath.EvalSymlinks(afero.FullBaseFsPath(s.base, "/")) + if err != nil { + return false, err + } + + target := afero.FullBaseFsPath(s.base, p) + resolved, err := filepath.EvalSymlinks(target) + // When target does not resolve, work out where the operation would actually + // land. A non-existent regular path resolves to the file that would be + // created inside its containing directory, so walk up to the nearest + // existing ancestor and validate that. But when target itself is a dangling + // symlink, follow it one level instead: validating its lexical parent would + // wrongly accept a link pointing outside the scope, letting a write follow + // the link and create the file out of bounds. + for hops := 0; errors.Is(err, fs.ErrNotExist); { + if fi, lerr := os.Lstat(target); lerr == nil && fi.Mode()&os.ModeSymlink != 0 { + hops++ + if hops > maxSymlinkHops { + return false, os.ErrPermission + } + dest, rerr := os.Readlink(target) + if rerr != nil { + return false, rerr + } + if !filepath.IsAbs(dest) { + // Resolve the link relative to the directory that really contains + // it, not its lexical parent: a symlinked ancestor could otherwise + // shift the computed target back into scope while the real write + // lands outside it. The parent is guaranteed to resolve here + // because os.Lstat above already traversed it. + base, berr := filepath.EvalSymlinks(filepath.Dir(target)) + if berr != nil { + return false, berr + } + dest = filepath.Join(base, dest) + } + target = filepath.Clean(dest) + } else { + parent := filepath.Dir(target) + if parent == target { + break + } + target = parent + } + resolved, err = filepath.EvalSymlinks(target) + } + if err != nil { + return false, err + } + + // Compare against root with a trailing separator so a sibling like + // "/srvother" is not treated as being inside "/srv". When root is itself the + // filesystem boundary (e.g. "/"), it already ends in a separator, so avoid + // producing "//" — which no path would match — and accept any path under it. + prefix := root + if !strings.HasSuffix(prefix, string(filepath.Separator)) { + prefix += string(filepath.Separator) + } + + return resolved == root || strings.HasPrefix(resolved, prefix), nil +} + +func (s *ScopedFs) Create(name string) (afero.File, error) { + if err := s.guard(name); err != nil { + return nil, err + } + return s.base.Create(name) +} + +func (s *ScopedFs) Mkdir(name string, perm os.FileMode) error { + if err := s.guard(name); err != nil { + return err + } + return s.base.Mkdir(name, perm) +} + +func (s *ScopedFs) MkdirAll(path string, perm os.FileMode) error { + if err := s.guard(path); err != nil { + return err + } + return s.base.MkdirAll(path, perm) +} + +func (s *ScopedFs) Open(name string) (afero.File, error) { + if err := s.guard(name); err != nil { + return nil, err + } + return s.base.Open(name) +} + +func (s *ScopedFs) OpenFile(name string, flag int, perm os.FileMode) (afero.File, error) { + if err := s.guard(name); err != nil { + return nil, err + } + return s.base.OpenFile(name, flag, perm) +} + +func (s *ScopedFs) Remove(name string) error { + if err := s.guard(name); err != nil { + return err + } + return s.base.Remove(name) +} + +func (s *ScopedFs) RemoveAll(path string) error { + if err := s.guard(path); err != nil { + return err + } + return s.base.RemoveAll(path) +} + +func (s *ScopedFs) Rename(oldname, newname string) error { + if err := s.guard(oldname); err != nil { + return err + } + if err := s.guard(newname); err != nil { + return err + } + return s.base.Rename(oldname, newname) +} + +func (s *ScopedFs) Stat(name string) (os.FileInfo, error) { + if err := s.guard(name); err != nil { + return nil, err + } + return s.base.Stat(name) +} + +func (s *ScopedFs) Name() string { return "ScopedFs" } + +func (s *ScopedFs) Chmod(name string, mode os.FileMode) error { + if err := s.guard(name); err != nil { + return err + } + return s.base.Chmod(name, mode) +} + +func (s *ScopedFs) Chown(name string, uid, gid int) error { + if err := s.guard(name); err != nil { + return err + } + return s.base.Chown(name, uid, gid) +} + +func (s *ScopedFs) Chtimes(name string, atime, mtime time.Time) error { + if err := s.guard(name); err != nil { + return err + } + return s.base.Chtimes(name, atime, mtime) +} + +func (s *ScopedFs) LstatIfPossible(name string) (os.FileInfo, bool, error) { + if err := s.guard(name); err != nil { + return nil, false, err + } + return s.base.LstatIfPossible(name) +} diff --git a/fileutils/copy_test.go b/fileutils/copy_test.go new file mode 100644 index 00000000..6899e032 --- /dev/null +++ b/fileutils/copy_test.go @@ -0,0 +1,124 @@ +package fileutils + +import ( + "os" + "path" + "path/filepath" + "testing" + + "github.com/filebrowser/filebrowser/v2/files" + "github.com/spf13/afero" +) + +// failingOpenFs wraps an afero.Fs and makes Open fail for one specific path, +// while every other operation (including Stat) is delegated unchanged. It +// simulates a directory that can be stat-ed but not opened/read — for example +// an unreadable sub-directory, or one whose permissions changed or that was +// removed after its parent was listed (a TOCTOU race) — encountered during a +// recursive copy. +type failingOpenFs struct { + afero.Fs + failOpen string +} + +func (f *failingOpenFs) Open(name string) (afero.File, error) { + if path.Clean(name) == path.Clean(f.failOpen) { + return nil, os.ErrPermission + } + return f.Fs.Open(name) +} + +// CopyDir is documented to keep going when it hits an error and to report the +// error afterwards. A sub-directory that cannot be opened must therefore yield +// an error (and leave the other, readable entries copied) rather than +// panicking on a nil directory handle. +func TestCopyDirUnreadableSubdirReturnsError(t *testing.T) { + mem := afero.NewMemMapFs() + if err := mem.MkdirAll("/srcdir/sub", 0o755); err != nil { + t.Fatal(err) + } + if err := afero.WriteFile(mem, "/srcdir/ok.txt", []byte("readable"), 0o644); err != nil { + t.Fatal(err) + } + + afs := &failingOpenFs{Fs: mem, failOpen: "/srcdir/sub"} + + err := Copy(afs, "/srcdir", "/dstdir", 0o644, 0o755) + if err == nil { + t.Fatal("expected an error when a sub-directory cannot be opened") + } + + // The readable sibling must still have been copied (continue-on-error). + data, readErr := afero.ReadFile(afs, "/dstdir/ok.txt") + if readErr != nil { + t.Fatalf("readable sibling was not copied: %v", readErr) + } + if string(data) != "readable" { + t.Fatalf("unexpected copied content: %q", string(data)) + } +} + +// Copying an in-scope directory that contains a symlink whose target escapes +// the user's scope must not dereference that symlink into the destination. +// Otherwise a scoped user could exfiltrate out-of-scope file content via the +// recursive copy path (GHSA-c2gv-wf5f-hjhh, an incomplete fix of +// GHSA-239w-m3h6-ch8v). +func TestCopyDoesNotDereferenceEscapingSymlink(t *testing.T) { + base := t.TempDir() + scope := filepath.Join(base, "scope") + if err := os.MkdirAll(filepath.Join(scope, "srcdir"), 0o755); err != nil { + t.Fatal(err) + } + + // A secret living outside the scope. + secret := filepath.Join(base, "secret.txt") + if err := os.WriteFile(secret, []byte("OUT-OF-SCOPE-SECRET"), 0o644); err != nil { + t.Fatal(err) + } + + // An escaping symlink planted inside the user's scope. + if err := os.Symlink(secret, filepath.Join(scope, "srcdir", "link.txt")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + + afs := files.NewScopedFs(afero.NewOsFs(), scope) + + err := Copy(afs, "/srcdir", "/dstdir", 0o644, 0o755) + if err == nil { + t.Fatal("expected copy of a directory containing an escaping symlink to fail") + } + + // The escaping symlink's target content must not have landed in scope. + if data, readErr := afero.ReadFile(afs, "/dstdir/link.txt"); readErr == nil { + t.Fatalf("escaping symlink was dereferenced into scope: got %q", string(data)) + } +} + +// A symlink whose target stays within scope is legitimate and must still be +// copied (dereferenced) so the fix does not over-block normal usage. +func TestCopyAllowsInScopeSymlink(t *testing.T) { + scope := t.TempDir() + if err := os.MkdirAll(filepath.Join(scope, "srcdir", "real"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(scope, "srcdir", "real", "f.txt"), []byte("in-scope"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(scope, "srcdir", "real", "f.txt"), filepath.Join(scope, "srcdir", "link.txt")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + + afs := files.NewScopedFs(afero.NewOsFs(), scope) + + if err := Copy(afs, "/srcdir", "/dstdir", 0o644, 0o755); err != nil { + t.Fatalf("expected copy of an in-scope symlink to succeed, got: %v", err) + } + + data, err := afero.ReadFile(afs, "/dstdir/link.txt") + if err != nil { + t.Fatalf("expected in-scope symlink to be copied, got: %v", err) + } + if string(data) != "in-scope" { + t.Fatalf("unexpected copied content: %q", string(data)) + } +} diff --git a/fileutils/dir.go b/fileutils/dir.go index e0b049db..4bd7c925 100644 --- a/fileutils/dir.go +++ b/fileutils/dir.go @@ -23,7 +23,12 @@ func CopyDir(afs afero.Fs, source, dest string, fileMode, dirMode fs.FileMode) e return err } - dir, _ := afs.Open(source) + dir, err := afs.Open(source) + if err != nil { + return err + } + defer dir.Close() + obs, err := dir.Readdir(-1) if err != nil { return err diff --git a/frontend/public/index.html b/frontend/public/index.html index 15ff375e..dc59f230 100644 --- a/frontend/public/index.html +++ b/frontend/public/index.html @@ -5,7 +5,7 @@ [{[ if .ReCaptcha -]}] diff --git a/frontend/src/App.vue b/frontend/src/App.vue index 25917f7d..543b50b8 100644 --- a/frontend/src/App.vue +++ b/frontend/src/App.vue @@ -1,5 +1,6 @@ @@ -9,6 +10,7 @@ import { ref, onMounted, watch } from "vue"; import { useI18n } from "vue-i18n"; import { setHtmlLocale } from "./i18n"; import { getMediaPreference, getTheme, setTheme } from "./utils/theme"; +import { name } from "./utils/constants"; const { locale } = useI18n(); @@ -31,3 +33,17 @@ watch(locale, (newValue) => { newValue && setHtmlLocale(newValue); }); + + diff --git a/frontend/src/components/files/ExtendedImage.vue b/frontend/src/components/files/ExtendedImage.vue index 88b78304..62b8d566 100644 --- a/frontend/src/components/files/ExtendedImage.vue +++ b/frontend/src/components/files/ExtendedImage.vue @@ -57,6 +57,9 @@ const container = ref(null); onMounted(() => { if (!decodeUTIF() && imgex.value !== null) { imgex.value.src = props.src; + imgex.value.alt = decodeURIComponent( + props.src.split("/").pop() || "preview" + ); } props.classList.forEach((className) => @@ -88,6 +91,9 @@ watch( () => { if (!decodeUTIF() && imgex.value !== null) { imgex.value.src = props.src; + imgex.value.alt = decodeURIComponent( + props.src.split("/").pop() || "preview" + ); } scale.value = 1; diff --git a/frontend/src/components/files/ListingItem.vue b/frontend/src/components/files/ListingItem.vue index d6747f59..d18233eb 100644 --- a/frontend/src/components/files/ListingItem.vue +++ b/frontend/src/components/files/ListingItem.vue @@ -18,7 +18,7 @@ :data-dir="isDir" :data-type="type" :aria-label="name" - :aria-selected="isSelected" + :aria-pressed="isSelected" :data-ext="getExtension(name).toLowerCase()" @contextmenu="contextMenu" > @@ -26,6 +26,7 @@ @@ -179,6 +180,7 @@ const drop = async (event: Event) => { to: props.url + encodeURIComponent(fileStore.req?.items[i].name), name: fileStore.req?.items[i].name, size: fileStore.req?.items[i].size, + isDir: fileStore.req?.items[i].isDir, modified: fileStore.req?.items[i].modified, overwrite: false, rename: false, @@ -204,7 +206,7 @@ const drop = async (event: Event) => { .catch($showError); }; - const conflict = await upload.checkConflict(items, path); + const conflict = await upload.checkConflict(items, path, true); if (conflict.length > 0) { layoutStore.showHover({ diff --git a/frontend/src/components/header/HeaderBar.vue b/frontend/src/components/header/HeaderBar.vue index d15ec060..bf281bcb 100644 --- a/frontend/src/components/header/HeaderBar.vue +++ b/frontend/src/components/header/HeaderBar.vue @@ -1,6 +1,6 @@