diff --git a/internal/builder/archive_symlink_test.go b/internal/builder/archive_symlink_test.go new file mode 100644 index 00000000..cf5d622b --- /dev/null +++ b/internal/builder/archive_symlink_test.go @@ -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) + } +} diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 153f414d..4e899c0b 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -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