Skip to content

Commit 0c7612b

Browse files
GustedGusted
authored andcommitted
fix: follow symlinks for local assets (#8596)
- This reverts behavior that was partially unintentionally introduced in forgejo/forgejo#8143, symbolic links were no longer followed (if they escaped the asset folder) for local assets. - Having symbolic links for user-added files is, to my understanding, a ,common usecase for NixOS and would thus have symbolic links in the asset folders. Avoiding symbolic links is not easy. - The previous code used `http.Dir`, we cannot use that as it's not of the same type. The equivalent is `os.DirFS`. - Unit test to prevent this regression from happening again. Reported-by: bloxx12 (Matrix). Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8596 Reviewed-by: Earl Warren <[email protected]> Co-authored-by: Gusted <[email protected]> Co-committed-by: Gusted <[email protected]>
1 parent 65a9c12 commit 0c7612b

File tree

2 files changed

+30
-9
lines changed

2 files changed

+30
-9
lines changed

modules/assetfs/layered.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,7 @@ func Local(name, base string, sub ...string) *Layer {
5656
panic(fmt.Sprintf("Unable to get absolute path for %q: %v", base, err))
5757
}
5858
root := util.FilePathJoinAbs(base, sub...)
59-
fsRoot, err := os.OpenRoot(root)
60-
if err != nil {
61-
if errors.Is(err, fs.ErrNotExist) {
62-
return nil
63-
}
64-
panic(fmt.Sprintf("Unable to open layer %q", err))
65-
}
66-
return &Layer{name: name, fs: fsRoot.FS(), localPath: root}
59+
return &Layer{name: name, fs: os.DirFS(root), localPath: root}
6760
}
6861

6962
// Bindata returns a new Layer with the given name, it serves files from the given bindata asset.
@@ -80,7 +73,7 @@ type LayeredFS struct {
8073

8174
// Layered returns a new LayeredFS with the given layers. The first layer is the top layer.
8275
func Layered(layers ...*Layer) *LayeredFS {
83-
return &LayeredFS{layers: slices.DeleteFunc(layers, func(layer *Layer) bool { return layer == nil })}
76+
return &LayeredFS{layers: layers}
8477
}
8578

8679
// Open opens the named file. The caller is responsible for closing the file.

modules/assetfs/layered_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// Copyright 2023 The Gitea Authors. All rights reserved.
2+
// Copyright 2025 The Forgejo Authors. All rights reserved.
23
// SPDX-License-Identifier: MIT
34

45
package assetfs
@@ -108,3 +109,30 @@ func TestLayered(t *testing.T) {
108109
assert.Equal(t, "l1", assets.GetFileLayerName("f1"))
109110
assert.Equal(t, "l2", assets.GetFileLayerName("f2"))
110111
}
112+
113+
// Allow layers to read symlink outside the layer root.
114+
func TestLayeredSymlink(t *testing.T) {
115+
dir := t.TempDir()
116+
dirl1 := filepath.Join(dir, "l1")
117+
require.NoError(t, os.MkdirAll(dirl1, 0o755))
118+
119+
// Open layer in dir/l1
120+
layer := Local("l1", dirl1)
121+
122+
// Create a file in dir/outside
123+
fileContents := []byte("I am outside the layer")
124+
require.NoError(t, os.WriteFile(filepath.Join(dir, "outside"), fileContents, 0o600))
125+
// Symlink dir/l1/outside to dir/outside
126+
require.NoError(t, os.Symlink(filepath.Join(dir, "outside"), filepath.Join(dirl1, "outside")))
127+
128+
// Open dir/l1/outside.
129+
f, err := layer.Open("outside")
130+
require.NoError(t, err)
131+
defer f.Close()
132+
133+
// Confirm it contains the output of dir/outside
134+
contents, err := io.ReadAll(f)
135+
require.NoError(t, err)
136+
137+
assert.Equal(t, fileContents, contents)
138+
}

0 commit comments

Comments
 (0)