From 706df19b439170fe34aca827e704f304241b3285 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Mon, 11 May 2026 03:34:48 +0000 Subject: [PATCH 1/9] [core-be-agent] fix(security#321): CWE-22 path traversal guards in loadWorkspaceEnv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two vulnerable call sites confirmed on origin/main: 1. org_helpers.go:loadWorkspaceEnv (line 101): filesDir from untrusted org YAML joined directly with orgBaseDir without traversal guard. A malicious filesDir like "../../../etc" escapes the org root and reads arbitrary files. 2. org_import.go:createWorkspaceTree (line 494): same pattern directly in the env-loading block — not covered by staging-targeted PR #345. Fix (both locations): call resolveInsideRoot(orgBaseDir, filesDir) before filepath.Join. On traversal detection, org_helpers.go returns an empty map (caller contract); org_import.go silently skips the workspace .env override (matches existing template-resolution pattern in the same function). Tests: org_helpers_test.go — 3 cases covering traversal rejection, workspace-override happy path, and empty filesDir edge case. Closes: molecule-core#362, molecule-core#321 Co-Authored-By: Claude Opus 4.7 --- .../internal/handlers/org_helpers.go | 13 ++- .../internal/handlers/org_helpers_test.go | 104 ++++++++++++++++++ .../internal/handlers/org_import.go | 7 +- 3 files changed, 122 insertions(+), 2 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..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_helpers_test.go b/workspace-server/internal/handlers/org_helpers_test.go new file mode 100644 index 00000000..c42ca0cd --- /dev/null +++ b/workspace-server/internal/handlers/org_helpers_test.go @@ -0,0 +1,104 @@ +package handlers + +import ( + "os" + "path/filepath" + "testing" +) + +// TestLoadWorkspaceEnv_RejectsTraversal asserts that loadWorkspaceEnv refuses +// to read workspace-specific .env files when filesDir contains CWE-22 traversal +// patterns (../../../etc, absolute paths, etc.). This is the primary security +// control for the ws.FilesDir attack surface in POST /org/import. + +func TestLoadWorkspaceEnv_RejectsTraversal(t *testing.T) { + tmp := t.TempDir() + orgRoot := filepath.Join(tmp, "my-org") + if err := os.Mkdir(orgRoot, 0o755); err != nil { + t.Fatal(err) + } + + cases := []struct { + name string + filesDir string + }{ + {"traversal_parent", "../../../etc"}, + {"traversal_deep", "../../../../../../../../../etc"}, + {"traversal_sibling", "../sibling"}, + {"traversal_mixed", "foo/../../bar"}, + {"absolute_path", "/etc/passwd"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // Write an org-level .env to confirm it loads even when the + // workspace .env is rejected. + orgEnv := filepath.Join(orgRoot, ".env") + if err := os.WriteFile(orgEnv, []byte("ORG_KEY=org-value\n"), 0o644); err != nil { + t.Fatal(err) + } + + got := loadWorkspaceEnv(orgRoot, tc.filesDir) + + // Org-level .env must be loaded regardless of workspace rejection. + if got["ORG_KEY"] != "org-value" { + t.Errorf("org-level .env not loaded: got %v", got) + } + // Traversal path must NOT have been read. + if val, ok := got["TRAVERSAL_KEY"]; ok { + t.Errorf("traversal escaped: got TRAVERSAL_KEY=%q", val) + } + }) + } +} + +// TestLoadWorkspaceEnv_HappyPath verifies that legitimate filesDir values +// resolve correctly and workspace .env overrides org-level values. + +func TestLoadWorkspaceEnv_HappyPath(t *testing.T) { + tmp := t.TempDir() + orgRoot := filepath.Join(tmp, "my-org") + wsDir := filepath.Join(orgRoot, "workspaces", "dev-workspace") + if err := os.MkdirAll(wsDir, 0o755); err != nil { + t.Fatal(err) + } + + orgEnv := filepath.Join(orgRoot, ".env") + wsEnv := filepath.Join(wsDir, ".env") + if err := os.WriteFile(orgEnv, []byte("ORG_KEY=org-val\nSHARED=org-wins\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(wsEnv, []byte("WS_KEY=ws-val\nSHARED=ws-wins\n"), 0o644); err != nil { + t.Fatal(err) + } + + got := loadWorkspaceEnv(orgRoot, filepath.Join("workspaces", "dev-workspace")) + + if got["ORG_KEY"] != "org-val" { + t.Errorf("org-level key missing: %v", got) + } + if got["WS_KEY"] != "ws-val" { + t.Errorf("workspace key missing: %v", got) + } + if got["SHARED"] != "ws-wins" { + t.Errorf("workspace should override org-level: got %v", got) + } +} + +// TestLoadWorkspaceEnv_EmptyFilesDirOnlyLoadsOrgLevel verifies that an empty +// filesDir only loads the org-level .env (no workspace override). + +func TestLoadWorkspaceEnv_EmptyFilesDir(t *testing.T) { + tmp := t.TempDir() + orgRoot := filepath.Join(tmp, "my-org") + if err := os.Mkdir(orgRoot, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(orgRoot, ".env"), []byte("KEY=only-org\n"), 0o644); err != nil { + t.Fatal(err) + } + + got := loadWorkspaceEnv(orgRoot, "") + if got["KEY"] != "only-org" { + t.Errorf("expected only-org, got %v", got) + } +} diff --git a/workspace-server/internal/handlers/org_import.go b/workspace-server/internal/handlers/org_import.go index 2e06479f..e521198e 100644 --- a/workspace-server/internal/handlers/org_import.go +++ b/workspace-server/internal/handlers/org_import.go @@ -490,8 +490,13 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX // 1. Org root .env (shared defaults) parseEnvFile(filepath.Join(orgBaseDir, ".env"), envVars) // 2. Workspace-specific .env (overrides) + // SECURITY: ws.FilesDir is untrusted YAML input — guard against CWE-22 + // traversal so a crafted filesDir like "../../../etc" cannot escape orgBaseDir. 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) + } + // Traversal rejection: silently skip — callers expect partial env on failure. } } // Store as workspace secrets via DB (encrypted if key is set, raw otherwise) From fd40700c43bb2af008d2d3d6b89a75c486bf43bb Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Mon, 11 May 2026 03:48:31 +0000 Subject: [PATCH 2/9] [ci skip false-positive] force re-run CI (runner stuck at infra#241) From f82033a3ca3209a1d611be20c2c264ad94643932 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Mon, 11 May 2026 03:52:40 +0000 Subject: [PATCH 3/9] [ci force] force fresh runner From 1f9042688eb7359e5f652cf9b3688f51c74d2e9a Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Mon, 11 May 2026 05:02:59 +0000 Subject: [PATCH 4/9] ci: install jq before sop-tier-check script runs Gitea Actions runners (ubuntu-latest) do not bundle jq. The sop-tier-check script uses jq for all JSON API parsing. Install jq before the script runs so sop-tier-check can pass. Uses direct binary download from GitHub releases (faster, more reliable than apt-get in containerized environments) with apt-get fallback and jq --version smoke test. Co-Authored-By: Claude Opus 4.7 --- .gitea/workflows/sop-tier-check.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.gitea/workflows/sop-tier-check.yml b/.gitea/workflows/sop-tier-check.yml index d4b74ed3..76750d50 100644 --- a/.gitea/workflows/sop-tier-check.yml +++ b/.gitea/workflows/sop-tier-check.yml @@ -77,6 +77,23 @@ jobs: # works if we never check out PR HEAD. Same SHA the workflow # itself was loaded from. ref: ${{ github.event.pull_request.base.sha }} + - name: Install jq + # Gitea Actions runners (ubuntu-latest label) do not bundle jq. + # The sop-tier-check script uses jq for all JSON API parsing. + # Install jq before the script runs so sop-tier-check can pass. + # + # Method: download binary directly from GitHub releases (faster and + # more reliable than apt-get in containerized environments). Falls + # back to apt-get if the download fails. The smoke test confirms + # jq is on PATH before the main script runs. + run: | + set -e + timeout 60 curl -sSL \ + "https://github.com/jqlang/jq/releases/download/jq-1.7.1/jq-linux-amd64" \ + -o /usr/local/bin/jq && chmod +x /usr/local/bin/jq \ + || apt-get update -qq && apt-get install -y -qq jq + jq --version + - name: Verify tier label + reviewer team membership env: # SOP_TIER_CHECK_TOKEN is the org-level secret for the From 93b7d9a88a6bfe6532fb194c9ee2f89b82d26a43 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Mon, 11 May 2026 01:22:02 +0000 Subject: [PATCH 5/9] fix(a2a_tools): add comment + test coverage for string-form error handling in delegate_task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Staging branch bea89ce4 introduced duplicate dead code after a `return` in the delegate_task error-handling block — the first occurrence was the correct fix (adding isinstance(err, str)), but the second occurrence (now unreachable) made the block fragile. Main already has the correct code; this branch adds an explanatory comment and regression tests. The non-tool delegate_task() in a2a_tools.py uses httpx.AsyncClient directly (not send_a2a_message) and must handle three A2A proxy error shapes: {"error": "plain string"} ← the bug fix: isinstance(err, str) {"error": {"message": "...", ...}} ← pre-existing path {"error": {"nested": "object"}} ← falls through to str(err) Adds TestDelegateTaskDirect: test_string_form_error_returns_error_message — regression for AttributeError test_dict_form_error_returns_error_message — pre-existing path still works test_success_returns_result_text — happy path still works Co-Authored-By: Claude Opus 4.7 --- workspace/builtin_tools/a2a_tools.py | 2 + workspace/tests/test_a2a_tools_impl.py | 99 ++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/workspace/builtin_tools/a2a_tools.py b/workspace/builtin_tools/a2a_tools.py index acdd15cb..d568ee40 100644 --- a/workspace/builtin_tools/a2a_tools.py +++ b/workspace/builtin_tools/a2a_tools.py @@ -77,6 +77,8 @@ async def delegate_task(workspace_id: str, task: str) -> str: return str(result) if isinstance(result, str) else "(no text)" elif "error" in data: err = data["error"] + # Handle both string-form errors ("error": "some string") + # and object-form errors ("error": {"message": "...", "code": ...}). msg = "" if isinstance(err, dict): msg = err.get("message", "") diff --git a/workspace/tests/test_a2a_tools_impl.py b/workspace/tests/test_a2a_tools_impl.py index 801eae80..690b3fc5 100644 --- a/workspace/tests/test_a2a_tools_impl.py +++ b/workspace/tests/test_a2a_tools_impl.py @@ -326,6 +326,105 @@ class TestToolDelegateTask: assert a2a_tools._peer_names.get("ws-nona000") is not None +# --------------------------------------------------------------------------- +# delegate_task (non-tool, direct httpx path — used by adapter templates) +# --------------------------------------------------------------------------- + +class TestDelegateTaskDirect: + + async def test_string_form_error_returns_error_message(self): + """The A2A proxy can return {"error": "plain string"}. Must not raise + AttributeError: 'str' object has no attribute 'get'.""" + import a2a_tools + + # Mock: discover succeeds, A2A POST returns a string-form error + mc = AsyncMock() + mc.__aenter__ = AsyncMock(return_value=mc) + mc.__aexit__ = AsyncMock(return_value=False) + + async def fake_post(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={"error": "peer workspace unreachable"}) + return r + + async def fake_get(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={"url": "http://peer.svc/a2a"}) + return r + + mc.post = fake_post + mc.get = fake_get + + with patch("a2a_tools.httpx.AsyncClient", return_value=mc): + result = await a2a_tools.delegate_task("ws-peer-123", "do a thing") + + assert "Error" in result + assert "peer workspace unreachable" in result + + async def test_dict_form_error_returns_error_message(self): + """{"error": {"message": "...", "code": ...}} — the pre-existing path.""" + import a2a_tools + + mc = AsyncMock() + mc.__aenter__ = AsyncMock(return_value=mc) + mc.__aexit__ = AsyncMock(return_value=False) + + async def fake_post(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={"error": {"message": "internal server error", "code": 500}}) + return r + + async def fake_get(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={"url": "http://peer.svc/a2a"}) + return r + + mc.post = fake_post + mc.get = fake_get + + with patch("a2a_tools.httpx.AsyncClient", return_value=mc): + result = await a2a_tools.delegate_task("ws-peer-456", "do a thing") + + assert "Error" in result + assert "internal server error" in result + + async def test_success_returns_result_text(self): + """Happy path: result with parts returns the first text part.""" + import a2a_tools + + mc = AsyncMock() + mc.__aenter__ = AsyncMock(return_value=mc) + mc.__aexit__ = AsyncMock(return_value=False) + + async def fake_post(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={ + "result": { + "parts": [{"kind": "text", "text": "Task done!"}] + } + }) + return r + + async def fake_get(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={"url": "http://peer.svc/a2a"}) + return r + + mc.post = fake_post + mc.get = fake_get + + with patch("a2a_tools.httpx.AsyncClient", return_value=mc): + result = await a2a_tools.delegate_task("ws-peer-789", "do a thing") + + assert result == "Task done!" + + # --------------------------------------------------------------------------- # tool_delegate_task_async # --------------------------------------------------------------------------- From f4e42c23b279f8ba5ad5e6176394bb638144657a Mon Sep 17 00:00:00 2001 From: "claude-ceo-assistant (Claude Opus 4.7 on Hongming's MacBook)" Date: Sun, 10 May 2026 23:00:10 -0700 Subject: [PATCH 6/9] Revert "ci: install jq before sop-tier-check script runs" This reverts commit 1f9042688eb7359e5f652cf9b3688f51c74d2e9a. --- .gitea/workflows/sop-tier-check.yml | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/.gitea/workflows/sop-tier-check.yml b/.gitea/workflows/sop-tier-check.yml index 76750d50..d4b74ed3 100644 --- a/.gitea/workflows/sop-tier-check.yml +++ b/.gitea/workflows/sop-tier-check.yml @@ -77,23 +77,6 @@ jobs: # works if we never check out PR HEAD. Same SHA the workflow # itself was loaded from. ref: ${{ github.event.pull_request.base.sha }} - - name: Install jq - # Gitea Actions runners (ubuntu-latest label) do not bundle jq. - # The sop-tier-check script uses jq for all JSON API parsing. - # Install jq before the script runs so sop-tier-check can pass. - # - # Method: download binary directly from GitHub releases (faster and - # more reliable than apt-get in containerized environments). Falls - # back to apt-get if the download fails. The smoke test confirms - # jq is on PATH before the main script runs. - run: | - set -e - timeout 60 curl -sSL \ - "https://github.com/jqlang/jq/releases/download/jq-1.7.1/jq-linux-amd64" \ - -o /usr/local/bin/jq && chmod +x /usr/local/bin/jq \ - || apt-get update -qq && apt-get install -y -qq jq - jq --version - - name: Verify tier label + reviewer team membership env: # SOP_TIER_CHECK_TOKEN is the org-level secret for the From aa49dbc72832d042ac54fd59e613a8b08a288bd7 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Mon, 11 May 2026 06:15:42 +0000 Subject: [PATCH 7/9] fix(handlers): add rows.Err() checks after rows.Next() loops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add deferred error checks following rows.Next() iteration in: - ListDelegations (delegation.go): log on error, continue serving results - org import reconcile orphan query (org.go): log + append to reconcileErrs Fixes the rows.Err() gap identified in the delegated rows.Err() check PR (#302, closed; replaced by this PR). Two additional files already had the check (activity.go, memories.go) — pattern applied consistently here. Co-Authored-By: Claude Opus 4.7 --- workspace-server/internal/handlers/delegation.go | 3 +++ workspace-server/internal/handlers/org.go | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/workspace-server/internal/handlers/delegation.go b/workspace-server/internal/handlers/delegation.go index 6761ec7e..e0d06b8b 100644 --- a/workspace-server/internal/handlers/delegation.go +++ b/workspace-server/internal/handlers/delegation.go @@ -645,6 +645,9 @@ func (h *DelegationHandler) ListDelegations(c *gin.Context) { } delegations = append(delegations, entry) } + if err := rows.Err(); err != nil { + log.Printf("ListDelegations rows.Err: %v", err) + } if delegations == nil { delegations = []map[string]interface{}{} diff --git a/workspace-server/internal/handlers/org.go b/workspace-server/internal/handlers/org.go index 8b5c4585..b93671dd 100644 --- a/workspace-server/internal/handlers/org.go +++ b/workspace-server/internal/handlers/org.go @@ -800,6 +800,10 @@ func (h *OrgHandler) Import(c *gin.Context) { orphanIDs = append(orphanIDs, orphanID) } } + if err := rows.Err(); err != nil { + log.Printf("Org import reconcile: orphan query rows.Err: %v", err) + reconcileErrs = append(reconcileErrs, fmt.Sprintf("orphan query rows.Err: %v", err)) + } rows.Close() for _, oid := range orphanIDs { From 8d4a9a184fc28b2614d9a0c4ac227b257ae73c13 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Mon, 11 May 2026 06:24:01 +0000 Subject: [PATCH 8/9] ci: re-trigger after runner stall Force a fresh sop-tier-check run to check if runners have recovered from infra#241 OOM cascade. Co-Authored-By: Claude Opus 4.7 From 150bf84b0b9b1cfc1864bd4c7b553080f404b181 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Mon, 11 May 2026 06:42:24 +0000 Subject: [PATCH 9/9] ci: re-trigger CI for fresh PR Co-Authored-By: Claude Opus 4.7