Refuse an archive of a module directory that is a link out of the workspace's tree (issue 1589)

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))
+}

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)
+	}
+}