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 <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
This commit is contained in:
Gusted 2025-07-22 15:02:47 +02:00 committed by Gusted
commit 0c7612bca4
2 changed files with 30 additions and 9 deletions

View file

@ -56,14 +56,7 @@ func Local(name, base string, sub ...string) *Layer {
panic(fmt.Sprintf("Unable to get absolute path for %q: %v", base, err)) panic(fmt.Sprintf("Unable to get absolute path for %q: %v", base, err))
} }
root := util.FilePathJoinAbs(base, sub...) root := util.FilePathJoinAbs(base, sub...)
fsRoot, err := os.OpenRoot(root) return &Layer{name: name, fs: os.DirFS(root), localPath: root}
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
return nil
}
panic(fmt.Sprintf("Unable to open layer %q", err))
}
return &Layer{name: name, fs: fsRoot.FS(), localPath: root}
} }
// Bindata returns a new Layer with the given name, it serves files from the given bindata asset. // Bindata returns a new Layer with the given name, it serves files from the given bindata asset.
@ -80,7 +73,7 @@ type LayeredFS struct {
// Layered returns a new LayeredFS with the given layers. The first layer is the top layer. // Layered returns a new LayeredFS with the given layers. The first layer is the top layer.
func Layered(layers ...*Layer) *LayeredFS { func Layered(layers ...*Layer) *LayeredFS {
return &LayeredFS{layers: slices.DeleteFunc(layers, func(layer *Layer) bool { return layer == nil })} return &LayeredFS{layers: layers}
} }
// Open opens the named file. The caller is responsible for closing the file. // Open opens the named file. The caller is responsible for closing the file.

View file

@ -1,4 +1,5 @@
// Copyright 2023 The Gitea Authors. All rights reserved. // Copyright 2023 The Gitea Authors. All rights reserved.
// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT // SPDX-License-Identifier: MIT
package assetfs package assetfs
@ -108,3 +109,30 @@ func TestLayered(t *testing.T) {
assert.Equal(t, "l1", assets.GetFileLayerName("f1")) assert.Equal(t, "l1", assets.GetFileLayerName("f1"))
assert.Equal(t, "l2", assets.GetFileLayerName("f2")) assert.Equal(t, "l2", assets.GetFileLayerName("f2"))
} }
// Allow layers to read symlink outside the layer root.
func TestLayeredSymlink(t *testing.T) {
dir := t.TempDir()
dirl1 := filepath.Join(dir, "l1")
require.NoError(t, os.MkdirAll(dirl1, 0o755))
// Open layer in dir/l1
layer := Local("l1", dirl1)
// Create a file in dir/outside
fileContents := []byte("I am outside the layer")
require.NoError(t, os.WriteFile(filepath.Join(dir, "outside"), fileContents, 0o600))
// Symlink dir/l1/outside to dir/outside
require.NoError(t, os.Symlink(filepath.Join(dir, "outside"), filepath.Join(dirl1, "outside")))
// Open dir/l1/outside.
f, err := layer.Open("outside")
require.NoError(t, err)
defer f.Close()
// Confirm it contains the output of dir/outside
contents, err := io.ReadAll(f)
require.NoError(t, err)
assert.Equal(t, fileContents, contents)
}