Compare commits

..

1 Commits

Author SHA1 Message Date
app-fe a75373074a fix(canvas): sortParentsBeforeChildren stable ordering + TIER_CONFIG string keys
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 11s
sop-tier-check / tier-check (pull_request) Successful in 12s
audit-force-merge / audit (pull_request) Has been skipped
sortParentsBeforeChildren: stable-order fix — visit true roots (parentId
undefined) before orphans (parentId references missing node). Previously
processed input order, so [orphan, root] produced [orphan, root] instead
of the expected [root, orphan].

TIER_CONFIG: use string keys ("1"…"4") in toHaveProperty calls.
Vitest's toHaveProperty is string-keyed; TypeScript strict mode with
noPropertyAccessFromIndexSignature rejects numeric literal keys on
Record<number, TIER_CONFIG_LEVEL>.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-11 03:37:57 +00:00
8 changed files with 22 additions and 245 deletions
-17
View File
@@ -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
@@ -55,10 +55,10 @@ describe("statusDotClass", () => {
describe("TIER_CONFIG", () => {
it("has entries for all four tier levels", () => {
expect(TIER_CONFIG).toHaveProperty(1);
expect(TIER_CONFIG).toHaveProperty(2);
expect(TIER_CONFIG).toHaveProperty(3);
expect(TIER_CONFIG).toHaveProperty(4);
expect(TIER_CONFIG).toHaveProperty("1");
expect(TIER_CONFIG).toHaveProperty("2");
expect(TIER_CONFIG).toHaveProperty("3");
expect(TIER_CONFIG).toHaveProperty("4");
});
it("each tier has label, color, and border fields", () => {
+16 -1
View File
@@ -34,7 +34,22 @@ export function sortParentsBeforeChildren<T extends { id: string; parentId?: str
visited.add(n.id);
out.push(n);
};
for (const n of nodes) visit(n);
// Stable-sort: visit nodes without a valid parent first (true roots → orphans → non-roots).
// This ensures roots appear before their children and orphans appear after true roots.
const trueRoots: T[] = []; // parentId undefined
const orphans: T[] = []; // parentId defined but parent missing
const nonRoots: T[] = []; // parentId defined and parent exists in set
for (const n of nodes) {
if (!n.parentId) {
trueRoots.push(n);
} else {
const parent = byId.get(n.parentId);
if (parent) nonRoots.push(n);
else orphans.push(n);
}
}
for (const n of [...trueRoots, ...orphans]) visit(n);
for (const n of nonRoots) visit(n);
return out;
}
@@ -91,10 +91,6 @@ 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 == "" {
@@ -102,14 +98,7 @@ func loadWorkspaceEnv(orgBaseDir, filesDir string) map[string]string {
}
parseEnvFile(filepath.Join(orgBaseDir, ".env"), envVars)
if filesDir != "" {
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)
parseEnvFile(filepath.Join(orgBaseDir, filesDir, ".env"), envVars)
}
return envVars
}
@@ -1,104 +0,0 @@
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,13 +490,8 @@ 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 != "" {
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.
parseEnvFile(filepath.Join(orgBaseDir, ws.FilesDir, ".env"), envVars)
}
}
// Store as workspace secrets via DB (encrypted if key is set, raw otherwise)
-2
View File
@@ -77,8 +77,6 @@ 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", "")
-99
View File
@@ -326,105 +326,6 @@ 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
# ---------------------------------------------------------------------------