fix: canonicalize root in safeJoin to handle symlinked storage roots

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
This commit is contained in:
Kyle Paul Zengo 2026-06-29 01:31:55 +00:00
parent f2bcb7e063
commit e345845c41
2 changed files with 60 additions and 1 deletions

View file

@ -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 {

View file

@ -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