forked from molecule-ai/molecule-core
Merge branch 'main' into feat/internal-219-phase-3-port-ci-yml
This commit is contained in:
commit
24fc943890
@ -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{}{}
|
||||
|
||||
@ -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 {
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
104
workspace-server/internal/handlers/org_helpers_test.go
Normal file
104
workspace-server/internal/handlers/org_helpers_test.go
Normal file
@ -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)
|
||||
}
|
||||
}
|
||||
@ -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)
|
||||
|
||||
@ -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", "")
|
||||
|
||||
@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Loading…
Reference in New Issue
Block a user