From 19b61729ac228a5bd493661397ea6baf74c2cc28 Mon Sep 17 00:00:00 2001 From: Molecule AI Fullstack Engineer Date: Sun, 10 May 2026 23:12:21 +0000 Subject: [PATCH] fix(security): CWE-22 path traversal guard in loadWorkspaceEnv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OFFSEC-003 / closes #330. `loadWorkspaceEnv` used `filepath.Join(orgBaseDir, filesDir, ".env")` without a resolveInsideRoot guard on filesDir — allowing malicious org YAML to read files outside the org root (e.g. filesDir: "../../../etc"). Two locations patched: 1. org_helpers.go:loadWorkspaceEnv — wrap filesDir with resolveInsideRoot before joining into the load path. On traversal rejection the org-root .env is still loaded; the traversal path is silently skipped. 2. org_import.go:createWorkspaceTree — same unguarded Join at line 494 was patched with the identical guard. resolveInsideRoot is already established in the codebase (used for template and files_dir elsewhere in org_import.go), so no new primitives are introduced. Added org_helpers_test.go covering: - Normal load of org-root + workspace .env (workspace overrides org) - Traversal paths (../../../etc etc.) are silently rejected - Non-existent workspace dir returns org-root vars only - Empty orgBaseDir returns empty map --- .../internal/handlers/org_helpers.go | 6 +- .../internal/handlers/org_helpers_test.go | 123 ++++++++++++++++++ .../internal/handlers/org_import.go | 6 +- 3 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 workspace-server/internal/handlers/org_helpers_test.go diff --git a/workspace-server/internal/handlers/org_helpers.go b/workspace-server/internal/handlers/org_helpers.go index 824fd2d7..354d6898 100644 --- a/workspace-server/internal/handlers/org_helpers.go +++ b/workspace-server/internal/handlers/org_helpers.go @@ -98,7 +98,11 @@ 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 { + return envVars // silently reject path traversal + } + parseEnvFile(filepath.Join(safeFilesDir, ".env"), envVars) } return envVars } diff --git a/workspace-server/internal/handlers/org_helpers_test.go b/workspace-server/internal/handlers/org_helpers_test.go new file mode 100644 index 00000000..60c1bd69 --- /dev/null +++ b/workspace-server/internal/handlers/org_helpers_test.go @@ -0,0 +1,123 @@ +package handlers + +import ( + "os" + "path/filepath" + "testing" +) + +// TestLoadWorkspaceEnv_OrgRootOnly verifies that loadWorkspaceEnv loads the +// org-root .env file even when filesDir is empty. +func TestLoadWorkspaceEnv_OrgRootOnly(t *testing.T) { + tmp := t.TempDir() + orgEnv := filepath.Join(tmp, ".env") + if err := os.WriteFile(orgEnv, []byte("ORG_KEY=org_value\n"), 0o644); err != nil { + t.Fatal(err) + } + + got := loadWorkspaceEnv(tmp, "") + if got["ORG_KEY"] != "org_value" { + t.Errorf("got %v, want ORG_KEY=org_value", got) + } +} + +// TestLoadWorkspaceEnv_WorkspaceOverrides verifies that a workspace-specific .env +// file (identified by filesDir) overrides org-root values. +func TestLoadWorkspaceEnv_WorkspaceOverrides(t *testing.T) { + tmp := t.TempDir() + + // Org root .env + if err := os.WriteFile(filepath.Join(tmp, ".env"), []byte("KEY=org_value\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Workspace-specific .env + wsDir := filepath.Join(tmp, "my-workspace") + if err := os.Mkdir(wsDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(wsDir, ".env"), []byte("KEY=ws_value\n"), 0o644); err != nil { + t.Fatal(err) + } + + got := loadWorkspaceEnv(tmp, "my-workspace") + if got["KEY"] != "ws_value" { + t.Errorf("got %v, want KEY=ws_value (workspace override)", got) + } +} + +// TestLoadWorkspaceEnv_TraversalRejected verifies that CWE-22 path traversal +// via filesDir is silently rejected — no file outside orgBaseDir is read. +// The org-root .env is still loaded; only the traversal path is blocked. +func TestLoadWorkspaceEnv_TraversalRejected(t *testing.T) { + tmp := t.TempDir() + + // Org root .env — must still be loaded + if err := os.WriteFile(filepath.Join(tmp, ".env"), []byte("SAFE=safe_val\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Simulate a malicious YAML: filesDir = "../../../tmp/traversal" + // The orgBaseDir is tmp; the traversal target is /tmp/traversal/. + // resolveInsideRoot must reject this and loadWorkspaceEnv must silently + // return only the org-root vars. + got := loadWorkspaceEnv(tmp, "../../../tmp/traversal") + + if got["SAFE"] != "safe_val" { + t.Errorf("org-root .env should still be loaded, got %v", got) + } + // The traversal path is rejected, so no secrets from there appear. + if v, ok := got["TRAVERSAL_SECRET"]; ok { + t.Errorf("traversal path should not be read, got TRAVERSAL_SECRET=%q", v) + } + // Confirm the map contains only the safe key. + if len(got) != 1 || got["SAFE"] != "safe_val" { + t.Errorf("got %v, want only {SAFE:safe_val}", got) + } +} + +// TestLoadWorkspaceEnv_TraversalDotdotdot verifies multiple-dot traversal is +// blocked (e.g. "../../../etc"). +func TestLoadWorkspaceEnv_TraversalDotdotdot(t *testing.T) { + tmp := t.TempDir() + if err := os.WriteFile(filepath.Join(tmp, ".env"), []byte("X=x\n"), 0o644); err != nil { + t.Fatal(err) + } + cases := []string{ + "../../../etc", + "../../../../../../../../../etc/passwd", + "..\\..\\..\\windows\\system32", + } + for _, tc := range cases { + got := loadWorkspaceEnv(tmp, tc) + // Only org root env should be present + if len(got) != 1 || got["X"] != "x" { + t.Errorf("traversal %q: got %v, want only {X:x}", tc, got) + } + } +} + +// TestLoadWorkspaceEnv_NonExistentWorkspace returns org-root vars only when +// the workspace directory does not exist. +func TestLoadWorkspaceEnv_NonExistentWorkspace(t *testing.T) { + tmp := t.TempDir() + if err := os.WriteFile(filepath.Join(tmp, ".env"), []byte("A=a\n"), 0o644); err != nil { + t.Fatal(err) + } + got := loadWorkspaceEnv(tmp, "nonexistent-workspace") + if got["A"] != "a" { + t.Errorf("got %v, want A=a", got) + } + if len(got) != 1 { + t.Errorf("got %v, want only {A:a}", got) + } +} + +// TestLoadWorkspaceEnv_EmptyOrgBaseDir returns empty map when orgBaseDir is +// empty (no org root .env to load). +func TestLoadWorkspaceEnv_EmptyOrgBaseDir(t *testing.T) { + got := loadWorkspaceEnv("", "any-files-dir") + if len(got) != 0 { + t.Errorf("got %v, want empty map", got) + } +} diff --git a/workspace-server/internal/handlers/org_import.go b/workspace-server/internal/handlers/org_import.go index 2e06479f..96315905 100644 --- a/workspace-server/internal/handlers/org_import.go +++ b/workspace-server/internal/handlers/org_import.go @@ -489,9 +489,11 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX if orgBaseDir != "" { // 1. Org root .env (shared defaults) parseEnvFile(filepath.Join(orgBaseDir, ".env"), envVars) - // 2. Workspace-specific .env (overrides) + // 2. Workspace-specific .env (overrides) — guard against traversal if ws.FilesDir != "" { - parseEnvFile(filepath.Join(orgBaseDir, ws.FilesDir, ".env"), envVars) + if safeFilesDir, err := resolveInsideRoot(orgBaseDir, ws.FilesDir); err == nil { + parseEnvFile(filepath.Join(safeFilesDir, ".env"), envVars) + } } } // Store as workspace secrets via DB (encrypted if key is set, raw otherwise)