From c8718f25eae12c35477d37517d3c85213e5b5610 Mon Sep 17 00:00:00 2001
From: Kyle Zengo <zengokp@gmail.com>
Date: Wed, 1 Jul 2026 04:10:20 -0400
Subject: [PATCH] fix: canonicalize root in safeJoin to handle symlinked
 storage roots (#64)
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

Co-authored-by: Kyle Paul Zengo <kylezengo@mac2012kylezengo.tail2f018b.ts.net>
---
 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 d4e78ca..c73e81a 100644
--- a/internal/files/files_test.go
+++ b/internal/files/files_test.go
@@ -246,6 +246,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 12af575..1e316fe 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