From 851fcbfa328f72343a9184fb706672a3fe050efe Mon Sep 17 00:00:00 2001
From: Molecule AI Core-DevOps <core-devops@agents.moleculesai.app>
Date: Tue, 12 May 2026 08:28:35 +0000
Subject: [PATCH] fix: resolveInsideRoot uses filepath.EvalSymlinks to close
 CWE-59
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The pre-existing resolveInsideRoot (org_helpers.go) used only
filepath.Abs, which does NOT resolve symlinks on Unix. A symlink
planted inside the workspace that points outside (e.g.
workspaces/dev/leaked → /etc) passed the lexical prefix check
because /tmp/.../workspaces/dev/leaked lexically starts with
/tmp/.../.

Add filepath.EvalSymlinks after the lexical check:
1. Lexical check catches obvious .. escapes.
2. EvalSymlinks resolves symlinks; fails on broken symlinks.
3. Re-check the resolved path against absRoot — catches planted
   outbound symlinks (CWE-59).

Broken symlinks are rejected because EvalSymlinks returns an error,
which propagates as "symlink resolve failed". This matches the
regression test added in this PR.

Without this fix, TestResolveInsideRoot_RejectsSymlinkTraversal (the
CWE-59 regression test added alongside) FAILS on any Unix system
where /tmp is a real directory (symlink test returns nil instead of
error), causing CI/Platform (Go) to fail and blocking the
continue-on-error unmask needed for Phase 4.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 .../internal/handlers/org_helpers.go          | 32 ++++++++++++++-----
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/workspace-server/internal/handlers/org_helpers.go b/workspace-server/internal/handlers/org_helpers.go
index 24c973f8..665a14b9 100644
--- a/workspace-server/internal/handlers/org_helpers.go
+++ b/workspace-server/internal/handlers/org_helpers.go
@@ -319,15 +319,16 @@ 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.
+// resolveInsideRoot joins `userPath` onto `root` and ensures the resolved
+// result stays inside root. Rejects absolute paths outright.
 //
 // 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").
+// After the lexical check, filepath.EvalSymlinks is called on the joined path
+// to resolve any symlinks; the final resolved path is then checked against
+// root. This closes CWE-59 (symlink-based path traversal): a symlink planted
+// inside the workspace that points outside the root is rejected, and broken
+// symlinks are also rejected since they cannot be valid org files.
 func resolveInsideRoot(root, userPath string) (string, error) {
 	if userPath == "" {
 		return "", fmt.Errorf("path is empty")
@@ -344,9 +345,24 @@ func resolveInsideRoot(root, userPath string) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("joined abs: %w", err)
 	}
-	// Allow exact-root match (rare but valid) and any descendant.
+	// Lexical check: reject obvious escapes before the more expensive symlink walk.
 	if absJoined != absRoot && !strings.HasPrefix(absJoined, absRoot+string(filepath.Separator)) {
 		return "", fmt.Errorf("path escapes root")
 	}
-	return absJoined, nil
+	// Resolve symlinks. EvalSymlinks also fails on broken symlinks — those
+	// cannot be valid org files, so they are rejected.
+	resolved, err := filepath.EvalSymlinks(absJoined)
+	if err != nil {
+		return "", fmt.Errorf("symlink resolve failed: %w", err)
+	}
+	// Re-check after symlink resolution: a symlink inside the root that points
+	// outside must be caught here (e.g. workspaces/dev/leaked → /etc).
+	resolvedAbs, err := filepath.Abs(resolved)
+	if err != nil {
+		return "", fmt.Errorf("resolved abs: %w", err)
+	}
+	if resolvedAbs != absRoot && !strings.HasPrefix(resolvedAbs, absRoot+string(filepath.Separator)) {
+		return "", fmt.Errorf("resolved path escapes root")
+	}
+	return resolvedAbs, nil
 }