Refuse an archive or a module directory that leads out of the repository through a link in the tree, so a repository cannot make the build pack files it does not hold (issue 1609)

2026-10-21 09:22:14 +00:00
parent 0205b8f708
commit fcca6ba9e0
2 changed files with 146 additions and 1 deletions
@@ -0,0 +1,104 @@
+package builder
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"git.novox.be/novox/mesh-sdk/go/catalogue"
+)
+
+// A link as a parent of an archive's `from` must not lead the build out of the module: the repository
+// is somebody else's, and a build that followed it would pack, register and serve any directory the
+// build account can read (issue 1609).
+func TestAnArchiveWhoseFromGoesThroughALinkOutOfTheModuleIsRefused(t *testing.T) {
+	outside := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(outside, "x"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(outside, "x", "secret"), []byte("s"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	tree := t.TempDir()
+	if err := os.Symlink(outside, filepath.Join(tree, "link")); err != nil {
+		t.Fatal(err)
+	}
+	r := &recorded{}
+	a := catalogue.Artifact{Name: "all", Kind: catalogue.ArtifactArchive, From: "link/x"}
+	_, err := one(context.Background(), nil, r, "m", tree, t.TempDir(), "c", "", a, nil, nil, "", Npmrc{}, nil,
+		nil, func(string, string, ...any) {})
+	if err == nil {
+		t.Fatal("an archive was packed through a link out of the module")
+	}
+	realOutside, _ := filepath.EvalSymlinks(filepath.Join(outside, "x"))
+	realTree, _ := filepath.EvalSymlinks(tree)
+	for _, want := range []string{realOutside, realTree, "link/x"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("the refusal %q does not name %q", err, want)
+		}
+	}
+	if len(r.archives) != 0 {
+		t.Errorf("a refused archive left %d published", len(r.archives))
+	}
+}
+
+// A link that stays inside the module is still followed, and its archive keeps its digest.
+func TestAnArchiveWhoseFromGoesThroughALinkInsideTheModuleIsPacked(t *testing.T) {
+	tree := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(tree, "real", "x"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(tree, "real", "x", "f"), []byte("f"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink("real", filepath.Join(tree, "link")); err != nil {
+		t.Fatal(err)
+	}
+	r := &recorded{}
+	a := catalogue.Artifact{Name: "all", Kind: catalogue.ArtifactArchive, From: "link/x"}
+	if _, err := one(context.Background(), nil, r, "m", tree, t.TempDir(), "c", "", a, nil, nil, "", Npmrc{}, nil,
+		nil, func(string, string, ...any) {}); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// A module directory that is itself a link out of the fetched tree is refused, naming both places.
+func TestAModuleDirectoryThatIsALinkOutOfTheRepositoryIsRefused(t *testing.T) {
+	outside := t.TempDir()
+	if err := os.WriteFile(filepath.Join(outside, ManifestName), []byte("{}"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	tree := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(tree, "modules"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(outside, filepath.Join(tree, "modules", "evil")); err != nil {
+		t.Fatal(err)
+	}
+	_, err := inside(tree, "modules/evil")
+	if err == nil {
+		t.Fatal("a module directory behind a link out of the repository was accepted")
+	}
+	realOutside, _ := filepath.EvalSymlinks(outside)
+	realTree, _ := filepath.EvalSymlinks(tree)
+	for _, want := range []string{realOutside, realTree, "modules/evil"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("the refusal %q does not name %q", err, want)
+		}
+	}
+	// A link that stays inside the repository, and a module directory not there, are left as they were.
+	if err := os.MkdirAll(filepath.Join(tree, "real"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink("real", filepath.Join(tree, "modules", "fine")); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := inside(tree, "modules/fine"); err != nil {
+		t.Errorf("a link inside the repository was refused: %v", err)
+	}
+	if _, err := inside(tree, "modules/none"); err != nil {
+		t.Errorf("an absent module directory was refused here: %v", err)
+	}
+}
@@ -943,6 +943,23 @@ func inside(tree, path string) (string, error) {
 		return "", fmt.Errorf(
 			"%q leaves the repository, and a build reads only its own tree", path)
 	}
+	// A link in the fetched tree must not lead the module out of it either (issue 1609): compare where
+	// both really are. A module directory that does not exist is left to the read of its manifest.
+	realTree, err := filepath.EvalSymlinks(tree)
+	if err != nil {
+		return "", err
+	}
+	realWithin, err := filepath.EvalSymlinks(within)
+	if errors.Is(err, os.ErrNotExist) {
+		return within, nil
+	}
+	if err != nil {
+		return "", err
+	}
+	if rel, err := filepath.Rel(realTree, realWithin); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
+		return "", fmt.Errorf("%q leads out of the repository through a link: it resolves to %s, and the repository is %s",
+			path, realWithin, realTree)
+	}
 	return within, nil
 }

@@ -1326,7 +1343,10 @@ func one(ctx context.Context, run Runner, publish Publisher,
 			}
 		} else {
 			src.ownHas(cleanEntry(a.From) + "/**")
-			body, err = pack(filepath.Join(tree, a.From))
+			var root string
+			if root, err = archiveRoot(tree, a); err == nil {
+				body, err = pack(root)
+			}
 		}
 		if err != nil {
 			return catalogue.Built{}, fmt.Errorf("%s: packing %s failed: %w", module, a.Name, err)
@@ -1343,6 +1363,27 @@ func one(ctx context.Context, run Runner, publish Publisher,
 		module, a.Name, a.Kind)
 }

+// archiveRoot is the directory an archive artifact is packed from, refused when a link in the fetched
+// tree leads out of the module. The repository is somebody else's: pack follows a link in a parent
+// component of `from`, so without this a committed link makes the build pack, register and serve any
+// directory the build account can read (issue 1609). Both are compared where they really are.
+func archiveRoot(tree string, a catalogue.Artifact) (string, error) {
+	root := filepath.Join(tree, a.From)
+	realTree, err := filepath.EvalSymlinks(tree)
+	if err != nil {
+		return "", err
+	}
+	realRoot, err := filepath.EvalSymlinks(root)
+	if err != nil {
+		return "", err
+	}
+	if rel, err := filepath.Rel(realTree, realRoot); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
+		return "", fmt.Errorf("the archive %s is taken from %q, which lies outside the module: it resolves to %s, and the module is %s",
+			a.Name, a.From, realRoot, realTree)
+	}
+	return root, nil
+}
+
 // pack tars and gzips a directory.
 //
 // **Deterministically**: entries sorted, and no timestamps, uid, gid or original names carried