From ae274541f4488605199f8241c2749a60378f4256 Mon Sep 17 00:00:00 2001 From: Molecule AI Fullstack Engineer Date: Wed, 13 May 2026 07:22:32 +0000 Subject: [PATCH] =?UTF-8?q?fix(org):=20CWE-22=20regression=20=E2=80=94=20r?= =?UTF-8?q?estore=20resolveInsideRoot=20guard=20in=20createWorkspaceTree?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mc#786: parseEnvFile(filepath.Join(orgBaseDir, ws.FilesDir, ".env")) was called without the resolveInsideRoot path-traversal guard. A malicious org YAML with filesDir: "../../../etc" could read arbitrary server files. Fix: replace the two-parseEnvFile block with a single loadWorkspaceEnv call. loadWorkspaceEnv already applies resolveInsideRoot to ws.FilesDir internally, closing the regression introduced when the guard was dropped from createWorkspaceTree. Also removes duplicate test declarations (TestHasUnresolvedVarRef_* from org_test.go and TestExtractResponseText_ResultNotMap from delegation_test.go) that blocked go build — the comprehensive versions live in *_pure_test.go / *_extract_response_text_test.go and were not cleaned up from the parent files after the fix/test-declarations merge. Co-Authored-By: Claude Opus 4.7 --- .../internal/handlers/delegation_test.go | 8 ++--- .../internal/handlers/org_import.go | 12 ++++--- .../internal/handlers/org_test.go | 36 ++----------------- 3 files changed, 12 insertions(+), 44 deletions(-) diff --git a/workspace-server/internal/handlers/delegation_test.go b/workspace-server/internal/handlers/delegation_test.go index 14c447bf..f64ea12e 100644 --- a/workspace-server/internal/handlers/delegation_test.go +++ b/workspace-server/internal/handlers/delegation_test.go @@ -1304,12 +1304,8 @@ func TestExtractResponseText_ValidJSONNoResult(t *testing.T) { } } -func TestExtractResponseText_ResultNotMap(t *testing.T) { - got := extractResponseText([]byte(`{"result": "just a string"}`)) - if got != `{"result": "just a string"}` { - t.Errorf("result is string: got %q, want raw body", got) - } -} +// TestExtractResponseText_* cases live in delegation_extract_response_text_test.go +// to keep pure-helper tests in their own file. func TestExtractResponseText_PartsTextKind(t *testing.T) { body := []byte(`{"result":{"parts":[{"kind":"text","text":"Hello from agent"}]}}`) diff --git a/workspace-server/internal/handlers/org_import.go b/workspace-server/internal/handlers/org_import.go index 2e06479f..2bf86ac8 100644 --- a/workspace-server/internal/handlers/org_import.go +++ b/workspace-server/internal/handlers/org_import.go @@ -487,11 +487,13 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX // timeout (caught 2026-05-08 right after dev-only org/import). loadPersonaEnvFile(ws.FilesDir, envVars) if orgBaseDir != "" { - // 1. Org root .env (shared defaults) - parseEnvFile(filepath.Join(orgBaseDir, ".env"), envVars) - // 2. Workspace-specific .env (overrides) - if ws.FilesDir != "" { - parseEnvFile(filepath.Join(orgBaseDir, ws.FilesDir, ".env"), envVars) + // Load org root and workspace-specific .env files. loadWorkspaceEnv + // applies resolveInsideRoot to ws.FilesDir, closing the CWE-22 / + // mc#786 path-traversal regression introduced when the guard was + // dropped from createWorkspaceTree. + workspaceEnv := loadWorkspaceEnv(orgBaseDir, ws.FilesDir) + for k, v := range workspaceEnv { + envVars[k] = v // workspace-specific overrides org root } } // Store as workspace secrets via DB (encrypted if key is set, raw otherwise) diff --git a/workspace-server/internal/handlers/org_test.go b/workspace-server/internal/handlers/org_test.go index 19dbece9..bb1a742e 100644 --- a/workspace-server/internal/handlers/org_test.go +++ b/workspace-server/internal/handlers/org_test.go @@ -354,39 +354,9 @@ func TestExpandWithEnv_UnsetVar(t *testing.T) { } } -func TestHasUnresolvedVarRef_NoVars(t *testing.T) { - if hasUnresolvedVarRef("plain text", "plain text") { - t.Error("plain text should not be flagged") - } -} - -func TestHasUnresolvedVarRef_LiteralDollar(t *testing.T) { - // "$5" is a literal price, not a var ref — should NOT be flagged - if hasUnresolvedVarRef("price: $5", "price: $5") { - t.Error("literal $5 should not be flagged as unresolved") - } -} - -func TestHasUnresolvedVarRef_Resolved(t *testing.T) { - // Original had ${VAR}, expanded to "value" — fully resolved - if hasUnresolvedVarRef("${VAR}", "value") { - t.Error("fully resolved var should not be flagged") - } -} - -func TestHasUnresolvedVarRef_Unresolved(t *testing.T) { - // Original had ${VAR}, expanded to "" — unresolved - if !hasUnresolvedVarRef("${VAR}", "") { - t.Error("unresolved var should be flagged") - } -} - -func TestHasUnresolvedVarRef_DollarVarSyntax(t *testing.T) { - // $VAR syntax (no braces) — also a real ref - if !hasUnresolvedVarRef("$MISSING_VAR", "") { - t.Error("$VAR syntax should be detected as ref when unresolved") - } -} +// TestHasUnresolvedVarRef_* cases live in org_helpers_pure_test.go to keep +// pure-helper tests in their own file. Keep TestExpandWithEnv_UnsetVar here +// since expandWithEnv is used across multiple org handlers. func eqStringSlice(a, b []string) bool { if len(a) != len(b) { -- 2.45.2