Merge pull request #106 from Molecule-AI/fix/org-import-path-traversal

fix(security): #103 — path-sanitize + admin-gate POST /org/import

platform/internal/handlers/org.go

@@ -196,7 +196,16 @@ func (h *OrgHandler) Import(c *gin.Context) {
 	var orgBaseDir string // base directory for files_dir resolution
 
 	if body.Dir != "" {
-		orgBaseDir = filepath.Join(h.orgDir, body.Dir)
+		// Reject traversal attempts — `dir` must resolve inside h.orgDir.
+		// Without this, `dir: "../../../etc"` gets joined into h.orgDir and
+		// filepath.Join's lexical cleanup resolves it outside the root,
+		// letting an unauthenticated caller probe arbitrary filesystem paths.
+		resolved, err := resolveInsideRoot(h.orgDir, body.Dir)
+		if err != nil {
+			c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("invalid dir: %v", err)})
+			return
+		}
+		orgBaseDir = resolved
 		orgFile := filepath.Join(orgBaseDir, "org.yaml")
 		data, err := os.ReadFile(orgFile)
 		if err != nil {
@@ -349,9 +358,12 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, defa
 		}
 		templatePath := ""
 		if ws.Template != "" {
-			tp := filepath.Join(h.configsDir, ws.Template)
-			if _, err := os.Stat(tp); err == nil {
-				templatePath = tp
+			// `template` comes from the uploaded YAML — treat as untrusted.
+			// Only accept paths that stay inside h.configsDir.
+			if tp, err := resolveInsideRoot(h.configsDir, ws.Template); err == nil {
+				if _, statErr := os.Stat(tp); statErr == nil {
+					templatePath = tp
+				}
 			}
 		}
 		if templatePath == "" {
@@ -367,9 +379,12 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, defa
 		// Copy files_dir contents on top (system-prompt.md, CLAUDE.md, skills/, etc.)
 		// Uses templatePath for CopyTemplateToContainer — runs AFTER configFiles are written
 		if ws.FilesDir != "" && orgBaseDir != "" {
-			filesPath := filepath.Join(orgBaseDir, ws.FilesDir)
-			if info, err := os.Stat(filesPath); err == nil && info.IsDir() {
-				templatePath = filesPath
+			// `files_dir` also comes from untrusted YAML. Join inside orgBaseDir
+			// (already validated above) and reject anything that escapes.
+			if filesPath, err := resolveInsideRoot(orgBaseDir, ws.FilesDir); err == nil {
+				if info, statErr := os.Stat(filesPath); statErr == nil && info.IsDir() {
+					templatePath = filesPath
+				}
 			}
 		}
 
@@ -815,3 +830,35 @@ func mergePlugins(defaultPlugins, wsPlugins []string) []string {
 	}
 	return out
 }
+
+// resolveInsideRoot joins `userPath` onto `root` and ensures the lexically
+// cleaned result stays inside root. Rejects absolute paths outright and
+// anything containing ".." that would escape the root.
+//
+// Both arguments are resolved to absolute paths via filepath.Abs before the
+// prefix check so a root passed as a relative path still works correctly.
+// Follows Go's standard pattern for SSRF-class path sanitization; using
+// strings.HasPrefix on an absolute-path pair plus the separator guard rejects
+// sibling directories that share a prefix (e.g. "/foo" vs "/foobar").
+func resolveInsideRoot(root, userPath string) (string, error) {
+	if userPath == "" {
+		return "", fmt.Errorf("path is empty")
+	}
+	if filepath.IsAbs(userPath) {
+		return "", fmt.Errorf("absolute paths are not allowed")
+	}
+	absRoot, err := filepath.Abs(root)
+	if err != nil {
+		return "", fmt.Errorf("root abs: %w", err)
+	}
+	joined := filepath.Join(absRoot, userPath)
+	absJoined, err := filepath.Abs(joined)
+	if err != nil {
+		return "", fmt.Errorf("joined abs: %w", err)
+	}
+	// Allow exact-root match (rare but valid) and any descendant.
+	if absJoined != absRoot && !strings.HasPrefix(absJoined, absRoot+string(filepath.Separator)) {
+		return "", fmt.Errorf("path escapes root")
+	}
+	return absJoined, nil
+}

platform/internal/handlers/org_path_test.go

@@ -0,0 +1,100 @@
+package handlers
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// Exercises resolveInsideRoot — the SSRF-class path sanitizer used by
+// POST /org/import for `dir` / `template` / `files_dir`. Issue #103.
+// The helper is the single chokepoint preventing `../../../etc` escape,
+// so it earns a dedicated test file.
+
+func TestResolveInsideRoot_HappyPath(t *testing.T) {
+	tmp := t.TempDir()
+	sub := filepath.Join(tmp, "molecule-dev")
+	if err := os.Mkdir(sub, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	got, err := resolveInsideRoot(tmp, "molecule-dev")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	// Compare via Abs to tolerate macOS /private symlink normalization.
+	wantAbs, _ := filepath.Abs(sub)
+	if got != wantAbs {
+		t.Errorf("got %q, want %q", got, wantAbs)
+	}
+}
+
+func TestResolveInsideRoot_RejectsTraversal(t *testing.T) {
+	tmp := t.TempDir()
+	cases := []string{
+		"../etc",
+		"../../etc/passwd",
+		"molecule-dev/../../..",
+		"../../../../../../../../../etc",
+	}
+	for _, tc := range cases {
+		t.Run(tc, func(t *testing.T) {
+			if _, err := resolveInsideRoot(tmp, tc); err == nil {
+				t.Errorf("expected error for %q, got nil", tc)
+			}
+		})
+	}
+}
+
+func TestResolveInsideRoot_RejectsAbsolute(t *testing.T) {
+	tmp := t.TempDir()
+	if _, err := resolveInsideRoot(tmp, "/etc/passwd"); err == nil {
+		t.Error("absolute path must be rejected")
+	}
+}
+
+func TestResolveInsideRoot_RejectsEmpty(t *testing.T) {
+	tmp := t.TempDir()
+	if _, err := resolveInsideRoot(tmp, ""); err == nil {
+		t.Error("empty path must be rejected")
+	}
+}
+
+// A path whose Abs shares a prefix with root but is NOT inside root must be
+// rejected. Catches the classic string-prefix bug where "/foo" matches
+// "/foobar".
+func TestResolveInsideRoot_RejectsPrefixSibling(t *testing.T) {
+	tmp := t.TempDir()
+	sibling := tmp + "-sibling"
+	if err := os.MkdirAll(sibling, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.RemoveAll(sibling) })
+
+	// Use a relative path that lexically resolves to the sibling directory.
+	up := "../" + filepath.Base(sibling)
+	if _, err := resolveInsideRoot(tmp, up); err == nil {
+		t.Errorf("sibling-prefix path %q must be rejected", up)
+	}
+}
+
+func TestResolveInsideRoot_DeepSubpath(t *testing.T) {
+	tmp := t.TempDir()
+	deep := filepath.Join(tmp, "a", "b", "c")
+	if err := os.MkdirAll(deep, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	got, err := resolveInsideRoot(tmp, "a/b/c")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	wantAbs, _ := filepath.Abs(deep)
+	if got != wantAbs {
+		t.Errorf("got %q want %q", got, wantAbs)
+	}
+	// Sanity: result is a strict descendant of root.
+	rootAbs, _ := filepath.Abs(tmp)
+	if !strings.HasPrefix(got, rootAbs+string(filepath.Separator)) {
+		t.Errorf("result %q is not inside %q", got, rootAbs)
+	}
+}

platform/internal/router/router.go

@@ -311,7 +311,11 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi
 	orgDir := findOrgDir(configsDir)
 	orgh := handlers.NewOrgHandler(wh, broadcaster, prov, channelMgr, configsDir, orgDir)
 	r.GET("/org/templates", orgh.ListTemplates)
-	r.POST("/org/import", orgh.Import)
+	// /org/import can create arbitrary workspaces from an uploaded YAML — it
+	// must be an admin-gated route. The handler also path-sanitizes
+	// `dir`/`template`/`files_dir` via resolveInsideRoot, but defence-in-
+	// depth keeps the route behind AdminAuth regardless.
+	r.POST("/org/import", middleware.AdminAuth(db.DB), orgh.Import)
 
 	// Channels (social integrations — Telegram, Slack, Discord, etc.)
 	chh := handlers.NewChannelHandler(channelMgr)