From 800d6624d543b76f039b76f729565bbde7e1ef27 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Wed, 21 Oct 2026 09:45:58 +0000 Subject: [PATCH] Refuse an archive of a module directory that is a link out of the workspace's tree (issue 1589) --- .../cmd/mesh-build-go/archive.go | 15 +++++++++- .../cmd/mesh-build-go/archive_test.go | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/modules/mesh-build-go/cmd/mesh-build-go/archive.go b/modules/mesh-build-go/cmd/mesh-build-go/archive.go index a83eeb3f..0cef5fde 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/archive.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/archive.go @@ -62,11 +62,18 @@ func (m *Module) buildArchive(ctx context.Context, workspace, module, dir string if err != nil { return nil, err } + realTree, err := filepath.EvalSymlinks(filepath.Join(m.Work, workspace, "tree")) + if err != nil { + return nil, err + } + if !within(realTree, realDir) { + return nil, fmt.Errorf("the module directory of %s resolves to %s, which lies outside the workspace's tree %s", art.Name, realDir, realTree) + } realRoot, err := filepath.EvalSymlinks(root) if err != nil { return nil, err } - if rel, err := filepath.Rel(realDir, realRoot); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + if !within(realDir, realRoot) { return nil, fmt.Errorf("the archive %s is taken from %q, which lies outside the module: it resolves to %s, and the module is %s", art.Name, art.From, realRoot, realDir) } return m.packInto(workspace, module, art.Name, root) @@ -287,3 +294,9 @@ func untar(stream io.Reader, into, within string) error { } return nil } + +// within says whether path is base or lies below it. +func within(base, path string) bool { + rel, err := filepath.Rel(base, path) + return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/archive_test.go b/modules/mesh-build-go/cmd/mesh-build-go/archive_test.go index aa4716a2..509668ed 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/archive_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/archive_test.go @@ -218,3 +218,32 @@ func TestAnArchiveBehindASymlinkedParentIsRefused(t *testing.T) { t.Errorf("an archive of outside files was left: %v", entries) } } + +// A module directory that is itself a link out of the workspace's tree is refused: its root and its directory both +// resolve outside, so only the tree tells them apart (review of issue 1589). +func TestAnArchiveOfALinkedModuleDirectoryIsRefused(t *testing.T) { + m, states, _ := archiveModule(t, "") + outside := t.TempDir() + if err := os.MkdirAll(filepath.Join(outside, "files"), 0o755); err != nil { + t.Fatal(err) + } + os.WriteFile(filepath.Join(outside, "files", "key"), []byte("private"), 0o600) + os.WriteFile(filepath.Join(outside, "module.json"), []byte(`{"module":"evil","version":"1","build":{"artifacts":[{"name":"files","kind":"archive","from":"files"}]}}`), 0o644) + if err := os.Symlink(outside, filepath.Join(m.Work, "ws1", "tree", "modules", "evil")); err != nil { + t.Fatal(err) + } + for _, tool := range m.Tools(m.StepTools()) { + if tool.Name == "build_go_build" { + if _, err := tool.Run(map[string]any{"workspace": "ws1", "module": "modules/evil", "artifact": "files"}); err != nil { + t.Fatal(err) + } + } + } + st := states.waitEnd(t) + if st.State != delivery.StepFailed || !strings.Contains(st.Reason, "outside") || !strings.Contains(st.Reason, outside) { + t.Errorf("the step ended %s: %q", st.State, st.Reason) + } + if entries, _ := os.ReadDir(filepath.Join(m.Work, "ws1", "artifacts")); len(entries) != 0 { + t.Errorf("an archive of outside files was left: %v", entries) + } +}