fix: canonicalize root in safeJoin to handle symlinked storage roots (#64)

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>
This commit is contained in:
Kyle Zengo 2026-07-01 04:10:20 -04:00 committed by GitHub
parent 3e28a57b5d
commit c8718f25ea
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 60 additions and 1 deletions

View file

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

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