From e345845c41241f3f0ab703f16a125508da70e0d5 Mon Sep 17 00:00:00 2001 From: Kyle Paul Zengo Date: Mon, 29 Jun 2026 01:31:55 +0000 Subject: [PATCH] fix: canonicalize root in safeJoin to handle symlinked storage roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the configured storage root (or a system temp dir on macOS where /var → /private/var) is reached through a symlink, filepath.EvalSymlinks on a child path resolves to the canonical form, but within() was comparing against the lexical root — causing valid paths to be rejected with "files: path escapes its area". Fix: resolve the root once with EvalSymlinks before the symlink guard loop, and compare resolved paths against the canonical root. The initial lexical containment check (line 108) still uses the original root so that the returned path keeps the caller's expected prefix. Adds two regression tests: - TestSafeJoinSymlinkedRoot: valid file under a symlinked root is accepted - TestSafeJoinChildSymlinkEscapeStillBlocked: escaping child symlink is still rejected Fixes #62 --- internal/files/files_test.go | 52 ++++++++++++++++++++++++++++++++++++ internal/files/fs.go | 9 ++++++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/internal/files/files_test.go b/internal/files/files_test.go index 6a06b53..975e070 100644 --- a/internal/files/files_test.go +++ b/internal/files/files_test.go @@ -194,6 +194,58 @@ func TestUsageCountsOwnedAreas(t *testing.T) { } } +func TestSafeJoinSymlinkedRoot(t *testing.T) { + // A storage root reached through a symlink must not cause valid child paths + // to be rejected. Before the fix, EvalSymlinks resolved the probe to the + // canonical path while within() still compared against the lexical root, + // producing a false-positive errEscape. + dir := t.TempDir() + realRoot := filepath.Join(dir, "real") + if err := os.MkdirAll(realRoot, 0o755); err != nil { + t.Fatal(err) + } + linkRoot := filepath.Join(dir, "link") + if err := os.Symlink(realRoot, linkRoot); err != nil { + t.Skipf("symlink unsupported: %v", err) + } + + // A normal file inside the real root, accessed via the symlinked root. + target := filepath.Join(realRoot, "notes.txt") + if err := os.WriteFile(target, []byte("hello"), 0o644); err != nil { + t.Fatal(err) + } + got, err := safeJoin(linkRoot, "notes.txt") + if err != nil { + t.Fatalf("safeJoin with symlinked root rejected valid path: %v", err) + } + if want := filepath.Join(linkRoot, "notes.txt"); got != want { + t.Errorf("safeJoin = %q, want %q", got, want) + } +} + +func TestSafeJoinChildSymlinkEscapeStillBlocked(t *testing.T) { + // A child symlink inside the area that points outside must still be + // rejected, even when the root itself is a symlink. + dir := t.TempDir() + realRoot := filepath.Join(dir, "real") + if err := os.MkdirAll(realRoot, 0o755); err != nil { + t.Fatal(err) + } + linkRoot := filepath.Join(dir, "link") + if err := os.Symlink(realRoot, linkRoot); err != nil { + t.Skipf("symlink unsupported: %v", err) + } + + // Plant an escaping symlink inside the (real) workspace. + escape := filepath.Join(realRoot, "escape") + if err := os.Symlink("/etc", escape); err != nil { + t.Skipf("symlink unsupported: %v", err) + } + if _, err := safeJoin(linkRoot, "escape/passwd"); err == nil { + t.Error("path through an escaping child symlink must be rejected") + } +} + func TestRevokeBlocksAndQuotaOverride(t *testing.T) { svc, st, u := newTestService(t) if err := st.SetFilesQuota(u.ID, 4096); err != nil { diff --git a/internal/files/fs.go b/internal/files/fs.go index 267a095..ad3dab2 100644 --- a/internal/files/fs.go +++ b/internal/files/fs.go @@ -108,12 +108,19 @@ func safeJoin(root, rel string) (string, error) { if !within(root, full) { return "", errEscape } + // Canonicalize the root so that a symlinked storage root (e.g. /var → + // /private/var on macOS, or an operator-configured symlink) doesn't cause + // false-positive escapes when EvalSymlinks resolves the full path. + canonRoot := root + if r, err := filepath.EvalSymlinks(root); err == nil { + canonRoot = r + } // Symlink guard: resolve the longest existing prefix and re-check. This // catches a symlink (created out-of-band) that points outside the area. probe := full for { if resolvedPath, err := filepath.EvalSymlinks(probe); err == nil { - if !within(root, resolvedPath) { + if !within(canonRoot, resolvedPath) { return "", errEscape } break