From fdcbf4814fca6f3b395543ee6c1ec67852c92f15 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Mon, 11 May 2026 02:09:33 +0000 Subject: [PATCH] fix(security#321): path traversal guard in loadWorkspaceEnv (CWE-22) --- .../internal/handlers/org_helpers.go | 13 ++- .../internal/handlers/org_path_test.go | 93 +++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/workspace-server/internal/handlers/org_helpers.go b/workspace-server/internal/handlers/org_helpers.go index 824fd2d7..24c973f8 100644 --- a/workspace-server/internal/handlers/org_helpers.go +++ b/workspace-server/internal/handlers/org_helpers.go @@ -91,6 +91,10 @@ func expandWithEnv(s string, env map[string]string) string { // loadWorkspaceEnv reads the org root .env and the workspace-specific .env // (workspace overrides org root). Used by both secret injection and channel // config expansion. +// +// SECURITY: filesDir is sourced from untrusted org YAML input (ws.FilesDir). +// resolveInsideRoot guard prevents path traversal (CWE-22) where a malicious +// filesDir like "../../../etc" could escape the org root. func loadWorkspaceEnv(orgBaseDir, filesDir string) map[string]string { envVars := map[string]string{} if orgBaseDir == "" { @@ -98,7 +102,14 @@ func loadWorkspaceEnv(orgBaseDir, filesDir string) map[string]string { } parseEnvFile(filepath.Join(orgBaseDir, ".env"), envVars) if filesDir != "" { - parseEnvFile(filepath.Join(orgBaseDir, filesDir, ".env"), envVars) + safeFilesDir, err := resolveInsideRoot(orgBaseDir, filesDir) + if err != nil { + // Reject traversal attempt silently — callers expect an empty map + // on any read failure. + log.Printf("loadWorkspaceEnv: rejecting filesDir %q: %v", filesDir, err) + return envVars + } + parseEnvFile(filepath.Join(safeFilesDir, ".env"), envVars) } return envVars } diff --git a/workspace-server/internal/handlers/org_path_test.go b/workspace-server/internal/handlers/org_path_test.go index 2ec707ff..d8f35d5f 100644 --- a/workspace-server/internal/handlers/org_path_test.go +++ b/workspace-server/internal/handlers/org_path_test.go @@ -98,3 +98,96 @@ func TestResolveInsideRoot_DeepSubpath(t *testing.T) { t.Errorf("result %q is not inside %q", got, rootAbs) } } + +// ─── loadWorkspaceEnv ─────────────────────────────────────────────────────── + +// writeEnv is a test helper that creates a file at path with KEY=VALUE content. +func writeEnv(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } +} + +func TestLoadWorkspaceEnv_LoadsOrgRootAndWorkspaceEnv(t *testing.T) { + tmp := t.TempDir() + writeEnv(t, filepath.Join(tmp, ".env"), "ORG_VAR=org_value\n") + writeEnv(t, filepath.Join(tmp, "ws-files", ".env"), "WS_VAR=ws_value\n") + + got := loadWorkspaceEnv(tmp, "ws-files") + if got["ORG_VAR"] != "org_value" { + t.Errorf("ORG_VAR: got %q, want %q", got["ORG_VAR"], "org_value") + } + if got["WS_VAR"] != "ws_value" { + t.Errorf("WS_VAR: got %q, want %q", got["WS_VAR"], "ws_value") + } +} + +func TestLoadWorkspaceEnv_WorkspaceOverridesOrg(t *testing.T) { + tmp := t.TempDir() + writeEnv(t, filepath.Join(tmp, ".env"), "SHARED=org\n") + writeEnv(t, filepath.Join(tmp, "ws", ".env"), "SHARED=ws\n") + + got := loadWorkspaceEnv(tmp, "ws") + if got["SHARED"] != "ws" { + t.Errorf("SHARED: got %q, want %q (workspace should override)", got["SHARED"], "ws") + } +} + +func TestLoadWorkspaceEnv_RejectsTraversal(t *testing.T) { + tmp := t.TempDir() + // Write a .env outside the org root to prove it is NOT loaded. + parentDir := filepath.Dir(tmp) + escapeTarget := filepath.Join(parentDir, "escape-target") + writeEnv(t, filepath.Join(escapeTarget, ".env"), "ESCAPED=should_not_be_loaded\n") + + got := loadWorkspaceEnv(tmp, "../escape-target") + if _, ok := got["ESCAPED"]; ok { + t.Error("ESCAPED key leaked — path traversal not blocked") + } +} + +func TestLoadWorkspaceEnv_RejectsDeepTraversal(t *testing.T) { + tmp := t.TempDir() + // Deep traversal: ".." repeated enough to escape tmp's parent. + parentDir := filepath.Dir(tmp) + deepTraversal := strings.Repeat("../", 10) + escapeTarget := filepath.Join(parentDir, "escape-deep") + writeEnv(t, filepath.Join(escapeTarget, ".env"), "DEEP=should_not_be_loaded\n") + + got := loadWorkspaceEnv(tmp, deepTraversal+"escape-deep") + if _, ok := got["DEEP"]; ok { + t.Error("DEEP key leaked from deep traversal") + } +} + +func TestLoadWorkspaceEnv_EmptyFilesDirLoadsOrgRootOnly(t *testing.T) { + tmp := t.TempDir() + writeEnv(t, filepath.Join(tmp, ".env"), "ONLY_ROOT=rootonly\n") + + got := loadWorkspaceEnv(tmp, "") + if got["ONLY_ROOT"] != "rootonly" { + t.Errorf("ONLY_ROOT: got %q, want %q", got["ONLY_ROOT"], "rootonly") + } +} + +func TestLoadWorkspaceEnv_NonExistentFilesDirIsSilent(t *testing.T) { + tmp := t.TempDir() + writeEnv(t, filepath.Join(tmp, ".env"), "ROOT=ok\n") + + // Must not error — missing filesDir is a silent no-op. + got := loadWorkspaceEnv(tmp, "this-dir-does-not-exist") + if got["ROOT"] != "ok" { + t.Errorf("ROOT: got %q, want %q", got["ROOT"], "ok") + } +} + +func TestLoadWorkspaceEnv_EmptyOrgBaseDirReturnsEmpty(t *testing.T) { + got := loadWorkspaceEnv("", "any-dir") + if len(got) != 0 { + t.Errorf("empty orgBaseDir should return empty map, got %d entries", len(got)) + } +} -- 2.45.2